digideskio / protobuf-c

Automatically exported from code.google.com/p/protobuf-c
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

GPE in unpack of malformed input #63

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. Create a packed buffer of a message with 3 repeated message fields (3 arrays 
of he same type) - withs IDs 1,2,3
2. Try to unpack it with an obviously non-compatible message that has a 
required string upfront with ID 1 followed by the same 3 repeated message 
fields with IDs 2,3,4
3. The <new-message>_unpack() call fails with GPE in protobuf-c.c in the 
function protobuf_c_message_free_unpacked() around line 2399 (protobuf-c-1.4) 
in the following code where arr is null (and is not checked):

          else if (desc->fields[f].type == PROTOBUF_C_TYPE_MESSAGE)
            {
              unsigned i;
              for (i = 0; i < n; i++)
                protobuf_c_message_free_unpacked (((ProtobufCMessage**)arr)[i], allocator);
            }

What is the expected output? What do you see instead?
It is expected that the protobuf-c code will protect itself against invalid 
input and would not crash. The test program does print:

$ ./test
Unpacking old with new protobuf
message 'tables': missing required field 'name'
Segmentation fault (core dumped)

But on its way to release already allocated memory, it fails because the 
construction did not complete successfully. It is clear the input is malformed 
but as a format that is used over the wire, it must protect itself or else it 
becomes a vulnerability.

What version of the product are you using? On what operating system?
protobuf-1.4 on Windows (running under cygwin)

Please provide any additional information below.
Attached reproduction code.

Original issue reported on code.google.com by dror.harari on 29 May 2011 at 11:27

Attachments:

GoogleCodeExporter commented 8 years ago
This issue was closed by revision r322.

Original comment by lahike...@gmail.com on 2 Nov 2011 at 3:12