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.
diff --git a/pb_encode.c b/pb_encode.c
index dc5a273..d9c8b2a 100644
--- a/pb_encode.c
+++ b/pb_encode.c
@@ -87,8 +87,11 @@
{
if (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))
@@ -615,6 +618,7 @@
static bool checkreturn pb_enc_bytes(pb_ostream_t *stream, const pb_field_t *field, const void *src)
{
const pb_bytes_array_t *bytes = (const pb_bytes_array_t*)src;
+ size_t allocsize;
if (src == NULL)
{
@@ -622,8 +626,9 @@
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..e1b696e
--- /dev/null
+++ b/tests/regression/issue_547/SConscript
@@ -0,0 +1,20 @@
+# 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")
+
+# Build and run test
+test = opts.Program(["test.c", "test.pb.c", "pb_encode_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..79166eb
--- /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()
+{
+ uint8_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];
+}
