diff options
| -rw-r--r-- | pb_decode.c | 50 | ||||
| -rw-r--r-- | tests/decode_unittests/decode_unittests.c | 6 | ||||
| -rw-r--r-- | tests/intsizes/intsizes_unittests.c | 14 |
3 files changed, 53 insertions, 17 deletions
diff --git a/pb_decode.c b/pb_decode.c index b5ec1ef0..5982c8e5 100644 --- a/pb_decode.c +++ b/pb_decode.c @@ -1072,53 +1072,75 @@ bool pb_decode_fixed64(pb_istream_t *stream, void *dest) static bool checkreturn pb_dec_varint(pb_istream_t *stream, const pb_field_t *field, void *dest) { uint64_t value; + int64_t svalue; + int64_t clamped; if (!pb_decode_varint(stream, &value)) return false; + /* See issue 97: Google's C++ protobuf allows negative varint values to + * be cast as int32_t, instead of the int64_t that should be used when + * encoding. Previous nanopb versions had a bug in encoding. In order to + * not break decoding of such messages, we cast <=32 bit fields to + * int32_t first to get the sign correct. + */ + if (field->data_size == 8) + svalue = (int64_t)value; + else + svalue = (int32_t)value; + switch (field->data_size) { - case 1: *(int8_t*)dest = (int8_t)value; break; - case 2: *(int16_t*)dest = (int16_t)value; break; - case 4: *(int32_t*)dest = (int32_t)value; break; - case 8: *(int64_t*)dest = (int64_t)value; break; + case 1: clamped = *(int8_t*)dest = (int8_t)svalue; break; + case 2: clamped = *(int16_t*)dest = (int16_t)svalue; break; + case 4: clamped = *(int32_t*)dest = (int32_t)svalue; break; + case 8: clamped = *(int64_t*)dest = svalue; break; default: PB_RETURN_ERROR(stream, "invalid data_size"); } + + if (clamped != svalue) + PB_RETURN_ERROR(stream, "integer too large"); return true; } static bool checkreturn pb_dec_uvarint(pb_istream_t *stream, const pb_field_t *field, void *dest) { - uint64_t value; + uint64_t value, clamped; if (!pb_decode_varint(stream, &value)) return false; switch (field->data_size) { - case 1: *(uint8_t*)dest = (uint8_t)value; break; - case 2: *(uint16_t*)dest = (uint16_t)value; break; - case 4: *(uint32_t*)dest = (uint32_t)value; break; - case 8: *(uint64_t*)dest = value; break; + case 1: clamped = *(uint8_t*)dest = (uint8_t)value; break; + case 2: clamped = *(uint16_t*)dest = (uint16_t)value; break; + case 4: clamped = *(uint32_t*)dest = (uint32_t)value; break; + case 8: clamped = *(uint64_t*)dest = value; break; default: PB_RETURN_ERROR(stream, "invalid data_size"); } + if (clamped != value) + PB_RETURN_ERROR(stream, "integer too large"); + return true; } static bool checkreturn pb_dec_svarint(pb_istream_t *stream, const pb_field_t *field, void *dest) { - int64_t value; + int64_t value, clamped; if (!pb_decode_svarint(stream, &value)) return false; switch (field->data_size) { - case 1: *(int8_t*)dest = (int8_t)value; break; - case 2: *(int16_t*)dest = (int16_t)value; break; - case 4: *(int32_t*)dest = (int32_t)value; break; - case 8: *(int64_t*)dest = value; break; + case 1: clamped = *(int8_t*)dest = (int8_t)value; break; + case 2: clamped = *(int16_t*)dest = (int16_t)value; break; + case 4: clamped = *(int32_t*)dest = (int32_t)value; break; + case 8: clamped = *(int64_t*)dest = value; break; default: PB_RETURN_ERROR(stream, "invalid data_size"); } + + if (clamped != value) + PB_RETURN_ERROR(stream, "integer too large"); return true; } diff --git a/tests/decode_unittests/decode_unittests.c b/tests/decode_unittests/decode_unittests.c index 8c12f1cd..47f0fbdb 100644 --- a/tests/decode_unittests/decode_unittests.c +++ b/tests/decode_unittests/decode_unittests.c @@ -123,16 +123,16 @@ int main() } { - pb_istream_t s = S("\x01\xFF\xFF\x03"); + pb_istream_t s = S("\x01\x00"); pb_field_t f = {1, PB_LTYPE_VARINT, 0, 0, 4, 0, 0}; uint32_t d; COMMENT("Test pb_dec_varint using uint32_t") TEST(pb_dec_varint(&s, &f, &d) && d == 1) /* Verify that no more than data_size is written. */ - d = 0; + d = 0xFFFFFFFF; f.data_size = 1; - TEST(pb_dec_varint(&s, &f, &d) && (d == 0xFF || d == 0xFF000000)) + TEST(pb_dec_varint(&s, &f, &d) && (d == 0xFFFFFF00 || d == 0x00FFFFFF)) } { diff --git a/tests/intsizes/intsizes_unittests.c b/tests/intsizes/intsizes_unittests.c index 29cc7ab0..189825f7 100644 --- a/tests/intsizes/intsizes_unittests.c +++ b/tests/intsizes/intsizes_unittests.c @@ -101,6 +101,20 @@ int main() INT32_MIN, 0, INT32_MIN, INT64_MIN, 0, INT64_MIN, true); + COMMENT("Test overflow detection"); + TEST_ROUNDTRIP(-129, 0, -128, + -32768, 0, -32768, + INT32_MIN, 0, INT32_MIN, + INT64_MIN, 0, INT64_MIN, false); + TEST_ROUNDTRIP(127, 256, 127, + 32767, 65535, 32767, + INT32_MAX, UINT32_MAX, INT32_MAX, + INT64_MAX, UINT64_MAX, INT64_MAX, false); + TEST_ROUNDTRIP(-128, 0, -128, + -32768, 0, -32769, + INT32_MIN, 0, INT32_MIN, + INT64_MIN, 0, INT64_MIN, false); + if (status != 0) fprintf(stdout, "\n\nSome tests FAILED!\n"); |