From 733ae77e2330f683b2dcfc9b28a260abc59fc1b6 Mon Sep 17 00:00:00 2001 From: Ilya Lipnitskiy Date: Sat, 20 Mar 2021 20:15:55 -0700 Subject: [PATCH] protobuf-c.c: fix packed repeated bool parsing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From https://developers.google.com/protocol-buffers/docs/proto3#updating: int32, uint32, int64, uint64, and bool are all compatible – this means you can change a field from one of these types to another without breaking forwards- or backwards-compatibility. If a number is parsed from the wire which doesn't fit in the corresponding type, you will get the same effect as if you had cast the number to that type in C++ (e.g. if a 64-bit number is read as an int32, it will be truncated to 32 bits). Until this fix, protobuf-c did not conform to the rule above when it came to deserializing non-boolean packed repeated data into a protobuf_c_boolean array. Fully scan the varint and use parse_boolean to truncate the resulting value. Fixes #440 --- Makefile.am | 16 ++++++++++++++++ protobuf-c/protobuf-c.c | 12 ++++++++---- t/issue440/.gitignore | 1 + t/issue440/issue440.c | 30 ++++++++++++++++++++++++++++++ t/issue440/issue440.proto | 9 +++++++++ 5 files changed, 64 insertions(+), 4 deletions(-) create mode 100644 t/issue440/.gitignore create mode 100644 t/issue440/issue440.c create mode 100644 t/issue440/issue440.proto diff --git a/Makefile.am b/Makefile.am index c4474cb..6b126fd 100644 --- a/Makefile.am +++ b/Makefile.am @@ -274,6 +274,22 @@ BUILT_SOURCES += \ t/issue389/issue389.pb-c.c t/issue389/issue389.pb-c.h EXTRA_DIST += \ t/issue389/issue389.proto + +check_PROGRAMS += \ + t/issue440/issue440 +TESTS += \ + t/issue440/issue440 +t_issue440_issue440_SOURCES = \ + t/issue440/issue440.c \ + t/issue440/issue440.pb-c.c +t_issue440_issue440_LDADD = \ + protobuf-c/libprotobuf-c.la +t/issue440/issue440.pb-c.c t/issue440/issue440.pb-c.h: $(top_builddir)/protoc-c/protoc-gen-c$(EXEEXT) $(top_srcdir)/t/issue440/issue440.proto + $(AM_V_GEN)@PROTOC@ --plugin=protoc-gen-c=$(top_builddir)/protoc-c/protoc-gen-c$(EXEEXT) -I$(top_srcdir) --c_out=$(top_builddir) $(top_srcdir)/t/issue440/issue440.proto +BUILT_SOURCES += \ + t/issue440/issue440.pb-c.c t/issue440/issue440.pb-c.h +EXTRA_DIST += \ + t/issue440/issue440.proto endif # BUILD_PROTO3 EXTRA_DIST += \ t/issue330/issue330.proto diff --git a/protobuf-c/protobuf-c.c b/protobuf-c/protobuf-c.c index aff3526..6dd7c3d 100644 --- a/protobuf-c/protobuf-c.c +++ b/protobuf-c/protobuf-c.c @@ -2748,7 +2748,9 @@ parse_packed_repeated_member(ScannedMember *scanned_member, const uint8_t *at = scanned_member->data + scanned_member->length_prefix_len; size_t rem = scanned_member->len - scanned_member->length_prefix_len; size_t count = 0; +#if defined(WORDS_BIGENDIAN) unsigned i; +#endif switch (field->type) { case PROTOBUF_C_TYPE_SFIXED32: @@ -2841,13 +2843,15 @@ parse_packed_repeated_member(ScannedMember *scanned_member, } break; case PROTOBUF_C_TYPE_BOOL: - count = rem; - for (i = 0; i < count; i++) { - if (at[i] > 1) { + while (rem > 0) { + unsigned s = scan_varint(rem, at); + if (s == 0) { PROTOBUF_C_UNPACK_ERROR("bad packed-repeated boolean value"); return FALSE; } - ((protobuf_c_boolean *) array)[i] = at[i]; + ((protobuf_c_boolean *) array)[count++] = parse_boolean(s, at); + at += s; + rem -= s; } break; default: diff --git a/t/issue440/.gitignore b/t/issue440/.gitignore new file mode 100644 index 0000000..ebba1c5 --- /dev/null +++ b/t/issue440/.gitignore @@ -0,0 +1 @@ +issue440 diff --git a/t/issue440/issue440.c b/t/issue440/issue440.c new file mode 100644 index 0000000..c3ca487 --- /dev/null +++ b/t/issue440/issue440.c @@ -0,0 +1,30 @@ +#include +#include + +#include "t/issue440/issue440.pb-c.h" + +int main(void) +{ + /* Output of $ echo "int: 1 int: -142342 int: 0 int: 423423222" | \ + * protoc issue440.proto --encode=Int | xxd -i: + * 0x0a, 0x11, 0x01, 0xfa, 0xa7, 0xf7, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + * 0x01, 0x00, 0xf6, 0xd9, 0xf3, 0xc9, 0x01 + * + * Output of $ echo "int: 1 int: -142342 int: 0 int: 423423222" | \ + * protoc issue440.proto --encode=Int | protoc issue440.proto \ + * --decode=Boolean: boolean: true boolean: true boolean: false boolean: true + */ + uint8_t protoc[] = {0x0a, 0x11, 0x01, 0xfa, 0xa7, 0xf7, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0x01, 0x00, 0xf6, 0xd9, 0xf3, 0xc9, + 0x01}; + Boolean *msg = boolean__unpack (NULL, sizeof protoc, protoc); + assert(msg); + assert(msg->n_boolean == 4); + assert(msg->boolean[0] == 1); + assert(msg->boolean[1] == 1); + assert(msg->boolean[2] == 0); + assert(msg->boolean[3] == 1); + boolean__free_unpacked (msg, NULL); + + return EXIT_SUCCESS; +} diff --git a/t/issue440/issue440.proto b/t/issue440/issue440.proto new file mode 100644 index 0000000..7e66d3a --- /dev/null +++ b/t/issue440/issue440.proto @@ -0,0 +1,9 @@ +syntax = "proto3"; + +message Int { + repeated int32 int = 1; +} + +message Boolean { + repeated bool boolean = 1; +}