commit e4f7b24204b21085e896dd2c40ffe3eccb1f297a
author Petteri Aimonen <jpa@git.mail.kapsi.fi> Mon Jul 13 08:44:17 2020 +0300
committer Petteri Aimonen <jpa@git.mail.kapsi.fi> Mon Jul 13 08:45:43 2020 +0300
tree 968ddcc2ec72e277635f9b38ea882588f122576e
parent 142d319c5e14d989d5a935ec96b13ea7943ff67f

Make pb_decode_varint32() reject overlong varint encodings.

This makes behavior more consistent with pb_decode_varint(),
read_raw_value() and protoc's implementation, which simplifies
testing.

pb_decode.c

diff --git a/pb_decode.c b/pb_decode.c
index 2dab0e3..4f11c9d 100644
--- a/pb_decode.c
+++ b/pb_decode.c
@@ -204,8 +204,10 @@
             {
                 /* Note: The varint could have trailing 0x80 bytes, or 0xFF for negative. */
                 pb_byte_t sign_extension = (bitpos < 63) ? 0xFF : 0x01;
-                
-                if ((byte & 0x7F) != 0x00 && ((result >> 31) == 0 || byte != sign_extension))
+                bool valid_extension = ((byte & 0x7F) == 0x00 ||
+                         ((result >> 31) != 0 && byte == sign_extension));
+
+                if (bitpos >= 64 || !valid_extension)
                 {
                     PB_RETURN_ERROR(stream, "varint overflow");
                 }

tests/decode_unittests/decode_unittests.c

diff --git a/tests/decode_unittests/decode_unittests.c b/tests/decode_unittests/decode_unittests.c
index 80b87b5..b876f3c 100644
--- a/tests/decode_unittests/decode_unittests.c
+++ b/tests/decode_unittests/decode_unittests.c
@@ -104,6 +104,7 @@
         TEST((s = S("\xFF\xFF\xFF\xFF\x10"), !pb_decode_varint32(&s, &u)));
         TEST((s = S("\xFF\xFF\xFF\xFF\x40"), !pb_decode_varint32(&s, &u)));
         TEST((s = S("\xFF\xFF\xFF\xFF\xFF\x01"), !pb_decode_varint32(&s, &u)));
+        TEST((s = S("\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\x80\x00"), !pb_decode_varint32(&s, &u)));
     }
 
     {