Fix buffer overflow when encoding bytes with size set to 65535 (#547)

On platforms where size_t equals pb_size_t, for example AVR where both
are 16-bit, or x86 and ARM when PB_FIELD_32BIT is defined, the buffer size
checks in pb_write() and pb_enc_submessage can overflow if a bytes field
has size close to maximum size value. This causes read and write out of bounds.

This issue can cause a security vulnerability if the size of a bytes field
in the structure given to pb_encode() is untrusted. Note that pb_decode()
has correct bounds checking and will reject too large values.

4 files changed, 64 insertions, 3 deletions

diff --git a/pb_encode.c b/pb_encode.c
index be82510..371d256 100644
--- a/pb_encode.c
+++ b/pb_encode.c
@@ -104,8 +104,11 @@ bool checkreturn pb_write(pb_ostream_t *stream, const pb_byte_t *buf, size_t cou
 {
     if (count > 0 && stream->callback != NULL)
     {
-        if (stream->bytes_written + count > stream->max_size)
+        if (stream->bytes_written + count < stream->bytes_written ||
+            stream->bytes_written + count > stream->max_size)
+        {
             PB_RETURN_ERROR(stream, "stream full");
+        }
 
 #ifdef PB_BUFFER_ONLY
         if (!buf_write(stream, buf, count))
@@ -849,6 +852,7 @@ static bool checkreturn pb_enc_fixed32(pb_ostream_t *stream, const pb_field_t *f
 static bool checkreturn pb_enc_bytes(pb_ostream_t *stream, const pb_field_t *field, const void *src)
 {
     const pb_bytes_array_t *bytes = NULL;
+    size_t allocsize;
 
     bytes = (const pb_bytes_array_t*)src;
     
@@ -858,8 +862,9 @@ static bool checkreturn pb_enc_bytes(pb_ostream_t *stream, const pb_field_t *fie
         return pb_encode_string(stream, NULL, 0);
     }
     
-    if (PB_ATYPE(field->type) == PB_ATYPE_STATIC &&
-        PB_BYTES_ARRAY_T_ALLOCSIZE(bytes->size) > field->data_size)
+    allocsize = PB_BYTES_ARRAY_T_ALLOCSIZE(bytes->size);
+    if (allocsize < bytes->size ||
+        (PB_ATYPE(field->type) == PB_ATYPE_STATIC && allocsize > field->data_size))
     {
         PB_RETURN_ERROR(stream, "bytes size exceeded");
     }
diff --git a/tests/regression/issue_547/SConscript b/tests/regression/issue_547/SConscript
new file mode 100644
index 0000000..c0a19fa
--- /dev/null
+++ b/tests/regression/issue_547/SConscript
@@ -0,0 +1,21 @@
+# Regression test for issue #547:
+# Buffer overflow when encoding bytes with size set to 65535
+
+Import("env")
+
+env.NanopbProto("test.proto")
+
+# Define the compilation options
+opts = env.Clone()
+opts.Append(CPPDEFINES = {'PB_FIELD_32BIT': 1})
+
+# Build new version of core
+strict = opts.Clone()
+strict.Append(CFLAGS = strict['CORECFLAGS'])
+strict.Object("pb_encode_fields32.o", "$NANOPB/pb_encode.c")
+strict.Object("pb_common_fields32.o", "$NANOPB/pb_common.c")
+
+# Build and run test
+test = opts.Program(["test.c", "test.pb.c", "pb_encode_fields32.o", "pb_common_fields32.o"])
+
+env.RunTest(test)
diff --git a/tests/regression/issue_547/test.c b/tests/regression/issue_547/test.c
new file mode 100644
index 0000000..9530302
--- /dev/null
+++ b/tests/regression/issue_547/test.c
@@ -0,0 +1,28 @@
+#include <string.h>
+#include <pb_encode.h>
+#include <unittests.h>
+#include "test.pb.h"
+
+int main()
+{
+    pb_byte_t buf[512];
+    MyMessage msg = MyMessage_init_zero;
+    pb_ostream_t stream = pb_ostream_from_buffer(buf, sizeof(buf));
+
+    msg.mybytes.size = 0xFFFFFFFF;
+    
+    if (pb_encode(&stream, MyMessage_fields, &msg))
+    {
+        fprintf(stderr, "Failure: expected pb_encode() to fail.\n");
+        return 1;
+    }
+    else if (strcmp(PB_GET_ERROR(&stream), "bytes size exceeded") != 0)
+    {
+        fprintf(stderr, "Unexpected encoding error: %s\n", PB_GET_ERROR(&stream));
+        return 2;
+    }
+    else
+    {
+        return 0;
+    }
+}
diff --git a/tests/regression/issue_547/test.proto b/tests/regression/issue_547/test.proto
new file mode 100644
index 0000000..56daa41
--- /dev/null
+++ b/tests/regression/issue_547/test.proto
@@ -0,0 +1,7 @@
+syntax = "proto2";
+
+import "nanopb.proto";
+
+message MyMessage {
+    required bytes mybytes = 1 [(nanopb).max_size = 512];
+}