BinaryToJsonString/BinaryToJsonStream returns OK on malformed unknown length-delimited field (Skip failure ignored)
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:
- Build protobuf v33.2.
- Use a schema like:
syntax = "proto3";
package p;
message M { int32 a = 1; }
- Call
google::protobuf::json::BinaryToJsonString()(orBinaryToJsonStream()) for typetype.googleapis.com/p.Mwith 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 varint0x80000000(= 2147483648 =INT_MAX + 1) -
08= tag for known fielda(field #1, varint) -
b9 0a= varint value1337
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.