Fix segfault with pointer fields and proto3_singular_msgs = true. (#504,#505)
If option proto3_singular_msgs was given and a proto3 submessage
contained a pointer field set to NULL, the encoder would crash with
null pointer dereference.
diff --git a/pb_encode.c b/pb_encode.c
index 409fec3..1a94202 100644
--- a/pb_encode.c
+++ b/pb_encode.c
@@ -264,7 +264,22 @@
}
/* Rest is proto3 singular fields */
- if (PB_LTYPE(type) == PB_LTYPE_BYTES)
+ if (PB_LTYPE(type) <= PB_LTYPE_LAST_PACKABLE)
+ {
+ /* Simple integer / float fields */
+ pb_size_t i;
+ const char *p = (const char*)field->pData;
+ for (i = 0; i < field->data_size; i++)
+ {
+ if (p[i] != 0)
+ {
+ return false;
+ }
+ }
+
+ return true;
+ }
+ else if (PB_LTYPE(type) == PB_LTYPE_BYTES)
{
const pb_bytes_array_t *bytes = (const pb_bytes_array_t*)field->pData;
return bytes->size == 0;
@@ -302,27 +317,29 @@
return true;
}
}
-
+ else if (PB_ATYPE(type) == PB_ATYPE_POINTER)
{
- /* Catch-all branch that does byte-per-byte comparison for zero value.
- *
- * This is for all pointer fields, and for static PB_LTYPE_VARINT,
- * UVARINT, SVARINT, FIXED32, FIXED64, EXTENSION fields, and also
- * callback fields. These all have integer or pointer value which
- * can be compared with 0.
- */
- pb_size_t i;
- const char *p = (const char*)field->pData;
- for (i = 0; i < field->data_size; i++)
- {
- if (p[i] != 0)
- {
- return false;
- }
- }
-
- return true;
+ return field->pData == NULL;
}
+ else if (PB_ATYPE(type) == PB_ATYPE_CALLBACK)
+ {
+ if (PB_LTYPE(type) == PB_LTYPE_EXTENSION)
+ {
+ const pb_extension_t *extension = *(const pb_extension_t* const *)field->pData;
+ return extension == NULL;
+ }
+ else if (field->descriptor->field_callback == pb_default_field_callback)
+ {
+ pb_callback_t *pCallback = (pb_callback_t*)field->pData;
+ return pCallback->funcs.encode == NULL;
+ }
+ else
+ {
+ return field->descriptor->field_callback == NULL;
+ }
+ }
+
+ return false; /* Not typically reached, safe default for weird special cases. */
}
/* Encode a field with static or pointer allocation, i.e. one whose data
diff --git a/tests/regression/issue_504/test.c b/tests/regression/issue_504/test.c
index 1ca86db..6c6693a 100644
--- a/tests/regression/issue_504/test.c
+++ b/tests/regression/issue_504/test.c
@@ -13,34 +13,65 @@
int status = 0;
uint8_t buffer[512] = {0};
int i;
- pb_ostream_t ostream;
- MyMessage msg = MyMessage_init_zero;
- char *pStr, *pStrAligned;
- ostream = pb_ostream_from_buffer(buffer, sizeof(buffer));
- /* copy STR to a malloced 0x100 aligned address */
- pStr = malloc(sizeof(STR) + ALIGN);
- pStrAligned = (char*)((uintptr_t)(pStr + ALIGN) & ~(ALIGN - 1));
- memcpy(pStrAligned, STR, sizeof(STR));
+ {
+ pb_ostream_t ostream;
+ MyMessage msg = MyMessage_init_zero;
+ char *pStr, *pStrAligned;
- msg.submessage.somestring = pStrAligned;
- printf("%p: '%s'\n", msg.submessage.somestring, msg.submessage.somestring);
+ COMMENT("Test for false negatives with pointer value low byte 0x00")
+ ostream = pb_ostream_from_buffer(buffer, sizeof(buffer));
- if (!pb_encode(&ostream, MyMessage_fields, &msg)) {
- fprintf(stderr, "Encode failed: %s\n", PB_GET_ERROR(&ostream));
- return 1;
+ /* copy STR to a malloced 0x100 aligned address */
+ pStr = malloc(sizeof(STR) + ALIGN);
+ pStrAligned = (char*)((uintptr_t)(pStr + ALIGN) & ~(ALIGN - 1));
+ memcpy(pStrAligned, STR, sizeof(STR));
+
+ msg.submessage.somestring = pStrAligned;
+ printf("%p: '%s'\n", msg.submessage.somestring, msg.submessage.somestring);
+
+ if (!pb_encode(&ostream, MyMessage_fields, &msg)) {
+ fprintf(stderr, "Encode failed: %s\n", PB_GET_ERROR(&ostream));
+ return 1;
+ }
+
+ free(pStr);
+ msg.submessage.somestring = NULL;
+
+ printf("response payload (%d):", (int)ostream.bytes_written);
+ for (i = 0; i < ostream.bytes_written; i++) {
+ printf("%02X", buffer[i]);
+ }
+ printf("\n");
+
+ TEST(ostream.bytes_written != 0);
}
- free(pStr);
- msg.submessage.somestring = NULL;
+ {
+ pb_ostream_t ostream;
+ struct {
+ MyMessage msg;
+ uint32_t bar;
+ } msg = {MyMessage_init_zero, 0};
- printf("response payload (%d):", (int)ostream.bytes_written);
- for (i = 0; i < ostream.bytes_written; i++) {
- printf("%02X", buffer[i]);
+ COMMENT("Test for false positives with data after end of struct")
+ ostream = pb_ostream_from_buffer(buffer, sizeof(buffer));
+
+ msg.bar = 0xFFFFFFFF;
+
+ if (!pb_encode(&ostream, MyMessage_fields, &msg)) {
+ fprintf(stderr, "Encode failed: %s\n", PB_GET_ERROR(&ostream));
+ return 1;
+ }
+
+ printf("response payload (%d):", (int)ostream.bytes_written);
+ for (i = 0; i < ostream.bytes_written; i++) {
+ printf("%02X", buffer[i]);
+ }
+ printf("\n");
+
+ TEST(ostream.bytes_written == 0);
}
- printf("\n");
-
- TEST(ostream.bytes_written != 0);
return status;
}
diff --git a/tests/regression/issue_504/test.proto b/tests/regression/issue_504/test.proto
index bdafd3c..7a6f0d4 100644
--- a/tests/regression/issue_504/test.proto
+++ b/tests/regression/issue_504/test.proto
@@ -10,4 +10,10 @@
message SubMessage
{
string somestring = 1 [(nanopb).type = FT_POINTER];
+ SubMessage2 submsg2 = 2 [(nanopb).type = FT_POINTER];
}
+
+message SubMessage2
+{
+ uint32 foo = 1;
+}
\ No newline at end of file
