diff --git a/Makefile.am b/Makefile.am index 6c836a3..aee2d84 100644 --- a/Makefile.am +++ b/Makefile.am @@ -250,6 +250,23 @@ BUILT_SOURCES += \ EXTRA_DIST += \ t/issue251/issue251.proto +# Issue #375 +check_PROGRAMS += \ + t/issue375/issue375 +TESTS += \ + t/issue375/issue375 +t_issue375_issue375_SOURCES = \ + t/issue375/issue375.c \ + t/issue375/issue375.pb-c.c +t_issue375_issue375_LDADD = \ + protobuf-c/libprotobuf-c.la +t/issue375/issue375.pb-c.c t/issue375/issue375.pb-c.h: $(top_builddir)/protoc-c/protoc-gen-c$(EXEEXT) $(top_srcdir)/t/issue375/issue375.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/issue375/issue375.proto +BUILT_SOURCES += \ + t/issue375/issue375.pb-c.c t/issue375/issue375.pb-c.h +EXTRA_DIST += \ + t/issue375/issue375.proto + endif # CROSS_COMPILING endif # BUILD_COMPILER diff --git a/protobuf-c/protobuf-c.c b/protobuf-c/protobuf-c.c index 5d857e1..c057870 100644 --- a/protobuf-c/protobuf-c.c +++ b/protobuf-c/protobuf-c.c @@ -2103,18 +2103,18 @@ struct _ScannedMember { const uint8_t *data; /**< Pointer to field data. */ }; -static inline uint32_t +static inline size_t scan_length_prefixed_data(size_t len, const uint8_t *data, size_t *prefix_len_out) { unsigned hdr_max = len < 5 ? len : 5; unsigned hdr_len; - uint32_t val = 0; + size_t val = 0; unsigned i; unsigned shift = 0; for (i = 0; i < hdr_max; i++) { - val |= (data[i] & 0x7f) << shift; + val |= ((size_t)data[i] & 0x7f) << shift; shift += 7; if ((data[i] & 0x80) == 0) break; @@ -2125,8 +2125,15 @@ scan_length_prefixed_data(size_t len, const uint8_t *data, } hdr_len = i + 1; *prefix_len_out = hdr_len; + if (val > INT_MAX) { + // Protobuf messages should always be less than 2 GiB in size. + // We also want to return early here so that hdr_len + val does + // not overflow on 32-bit systems. + PROTOBUF_C_UNPACK_ERROR("length prefix of %lu is too large", val); + return 0; + } if (hdr_len + val > len) { - PROTOBUF_C_UNPACK_ERROR("data too short after length-prefix of %u", val); + PROTOBUF_C_UNPACK_ERROR("data too short after length-prefix of %lu", val); return 0; } return hdr_len + val; diff --git a/t/issue375/.gitignore b/t/issue375/.gitignore new file mode 100644 index 0000000..4657e6d --- /dev/null +++ b/t/issue375/.gitignore @@ -0,0 +1 @@ +issue375 diff --git a/t/issue375/issue375.c b/t/issue375/issue375.c new file mode 100644 index 0000000..c808043 --- /dev/null +++ b/t/issue375/issue375.c @@ -0,0 +1,24 @@ +#include +#include + +#include "t/issue375/issue375.pb-c.h" + +int main(void) { + // This buffer represents some serialized bytes where we have a length + // delimiter of 2^32 - 1 bytes for a particular repeated int32 field. + // We want to make sure that parsing a length delimiter this large does + // not cause a problematic integer overflow. + uint8_t buffer[] = { + // Field 1 with wire type 2 (length-delimited) + 0x0a, + // Varint length delimiter: 2^32 - 1 + 0xff, 0xff, 0xff, 0xff, 0x0f, + }; + // The parser should detect that this message is malformed and return + // null. + Issue375__TestMessage* m = + issue375__test_message__unpack(NULL, sizeof(buffer), buffer); + assert(m == NULL); + + return EXIT_SUCCESS; +} diff --git a/t/issue375/issue375.proto b/t/issue375/issue375.proto new file mode 100644 index 0000000..0566fae --- /dev/null +++ b/t/issue375/issue375.proto @@ -0,0 +1,7 @@ +syntax = "proto2"; + +package issue375; + +message TestMessage { + repeated int32 nums = 1; +}