rik/nanopb
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Detect too large varint values when decoding.

Browse Source 
Because Issue #139 now allows limiting integer fields, it is good
to check the values received from other protobuf libraries against
the lower limits.
This commit is contained in:
Petteri Aimonen
2015-01-04 12:04:24 +02:00
parent 50c67ecec4
commit a0f0440394
3 changed files with 53 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

50
pb_decode.c
View File

@@ -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) static bool checkreturn pb_dec_varint(pb_istream_t *stream, const pb_field_t *field, void *dest)
{ {
uint64_t value; uint64_t value;
int64_t svalue;
int64_t clamped;
if (!pb_decode_varint(stream, &value)) if (!pb_decode_varint(stream, &value))
return false; 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) switch (field->data_size)
{ {
case 1: *(int8_t*)dest = (int8_t)value; break; case 1: clamped = *(int8_t*)dest = (int8_t)svalue; break;
case 2: *(int16_t*)dest = (int16_t)value; break; case 2: clamped = *(int16_t*)dest = (int16_t)svalue; break;
case 4: *(int32_t*)dest = (int32_t)value; break; case 4: clamped = *(int32_t*)dest = (int32_t)svalue; break;
case 8: *(int64_t*)dest = (int64_t)value; break; case 8: clamped = *(int64_t*)dest = svalue; break;
default: PB_RETURN_ERROR(stream, "invalid data_size"); default: PB_RETURN_ERROR(stream, "invalid data_size");
} }
if (clamped != svalue)
PB_RETURN_ERROR(stream, "integer too large");
return true; return true;
} }
static bool checkreturn pb_dec_uvarint(pb_istream_t *stream, const pb_field_t *field, void *dest) 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)) if (!pb_decode_varint(stream, &value))
return false; return false;
switch (field->data_size) switch (field->data_size)
{ {
case 1: *(uint8_t*)dest = (uint8_t)value; break; case 1: clamped = *(uint8_t*)dest = (uint8_t)value; break;
case 2: *(uint16_t*)dest = (uint16_t)value; break; case 2: clamped = *(uint16_t*)dest = (uint16_t)value; break;
case 4: *(uint32_t*)dest = (uint32_t)value; break; case 4: clamped = *(uint32_t*)dest = (uint32_t)value; break;
case 8: *(uint64_t*)dest = value; break; case 8: clamped = *(uint64_t*)dest = value; break;
default: PB_RETURN_ERROR(stream, "invalid data_size"); default: PB_RETURN_ERROR(stream, "invalid data_size");
} }
if (clamped != value)
PB_RETURN_ERROR(stream, "integer too large");
return true; return true;
} }
static bool checkreturn pb_dec_svarint(pb_istream_t *stream, const pb_field_t *field, void *dest) 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)) if (!pb_decode_svarint(stream, &value))
return false; return false;
switch (field->data_size) switch (field->data_size)
{ {
case 1: *(int8_t*)dest = (int8_t)value; break; case 1: clamped = *(int8_t*)dest = (int8_t)value; break;
case 2: *(int16_t*)dest = (int16_t)value; break; case 2: clamped = *(int16_t*)dest = (int16_t)value; break;
case 4: *(int32_t*)dest = (int32_t)value; break; case 4: clamped = *(int32_t*)dest = (int32_t)value; break;
case 8: *(int64_t*)dest = value; break; case 8: clamped = *(int64_t*)dest = value; break;
default: PB_RETURN_ERROR(stream, "invalid data_size"); default: PB_RETURN_ERROR(stream, "invalid data_size");
} }
if (clamped != value)
PB_RETURN_ERROR(stream, "integer too large");
return true; return true;
} }

6
tests/decode_unittests/decode_unittests.c
View File

@@ -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}; pb_field_t f = {1, PB_LTYPE_VARINT, 0, 0, 4, 0, 0};
uint32_t d; uint32_t d;
COMMENT("Test pb_dec_varint using uint32_t") COMMENT("Test pb_dec_varint using uint32_t")
TEST(pb_dec_varint(&s, &f, &d) && d == 1) TEST(pb_dec_varint(&s, &f, &d) && d == 1)
/* Verify that no more than data_size is written. */ /* Verify that no more than data_size is written. */
d = 0; d = 0xFFFFFFFF;
f.data_size = 1; 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))
} }
{ {

14
tests/intsizes/intsizes_unittests.c
View File

@@ -101,6 +101,20 @@ int main()
INT32_MIN, 0, INT32_MIN, INT32_MIN, 0, INT32_MIN,
INT64_MIN, 0, INT64_MIN, true); 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) if (status != 0)
fprintf(stdout, "\n\nSome tests FAILED!\n"); fprintf(stdout, "\n\nSome tests FAILED!\n");