protobuf icon indicating copy to clipboard operation
protobuf copied to clipboard

BinaryToJsonString/BinaryToJsonStream returns OK on malformed unknown length-delimited field (Skip failure ignored)

Open Ky0toFu opened this issue 1 month ago • 2 comments

What version of protobuf and what language are you using? Version: v33.2 (git tag commit 7ccf2060af4cd13c44a8659d0999be6c7a98fcc1) Language: C++

What operating system (Linux, Windows, ...) and version? Windows 11 with WSL Ubuntu 24.04 (also reproducible on Linux).

What runtime / compiler are you using (e.g., python version or gcc version) CMake build on WSL Ubuntu toolchain (gcc/clang). (I can provide exact versions if needed.)

What did you do? Steps to reproduce the behavior:

  1. Build protobuf v33.2.
  2. Use a schema like:
syntax = "proto3";
package p;
message M { int32 a = 1; }
  1. Call google::protobuf::json::BinaryToJsonString() (or BinaryToJsonStream()) for type type.googleapis.com/p.M with this crafted wire input (hex):

c2 3e 80 80 80 80 08 08 b9 0a

Interpretation:

  • c2 3e = unknown field tag (field #1000, wire type length-delimited)
  • 80 80 80 80 08 = length varint 0x80000000 (= 2147483648 = INT_MAX + 1)
  • 08 = tag for known field a (field #1, varint)
  • b9 0a = varint value 1337

What did you expect to see Conversion should fail (invalid/malformed wire input). The unknown length-delimited field length cannot be skipped and should be treated as an error.

What did you see instead? Conversion returns OK and produces “valid-looking” JSON (parse confusion), e.g. {"a":1337}.

Anything else we should know about your project / environment Root cause in src/google/protobuf/json/internal/untyped_message.cc (UntypedMessage::Decode()), unknown-field handling for WIRETYPE_LENGTH_DELIMITED:

uint32_t x;
if (!stream.ReadVarint32(&x)) {
  return MakeUnexpectedEofError();
}
stream.Skip(x);
continue;

CodedInputStream::Skip(int) takes an int. For x > INT_MAX, implicit conversion yields a negative value, Skip() returns false, but the return value is ignored so parsing continues.

Permalink (v33.2): https://github.com/protocolbuffers/protobuf/blob/7ccf2060af4cd13c44a8659d0999be6c7a98fcc1/src/google/protobuf/json/internal/untyped_message.cc#L254-L260

Suggested fix:

  • Reject x > INT_MAX.
  • Check the return value of Skip() and return an error when it fails.

Ky0toFu avatar Jan 01 '26 06:01 Ky0toFu