[Bro-Dev] [JIRA] (BIT-250) Binpac wrong boundary check

[Vlad Grigorescu]

Please ignore... Inadvertent mouse click. :-)


On Jun 2, 2014, at 8:05 PM, grigorescu (JIRA) <jira at bro-tracker.atlassian.net> wrote:

> 
>     [ https://bro-tracker.atlassian.net/browse/BIT-250?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
> 
> grigorescu updated BIT-250:
> ---------------------------
>    Status: Merge Request  (was: Open)
> 
>> Binpac wrong boundary check
>> ---------------------------
>> 
>>                Key: BIT-250
>>                URL: https://bro-tracker.atlassian.net/browse/BIT-250
>>            Project: Bro Issue Tracker
>>         Issue Type: Problem
>>         Components: BinPAC
>>   Affects Versions: 1.5.2
>>           Reporter: lorenzo simionato
>>        Attachments: test.pac
>> 
>> 
>> I'm trying to create a parser for a simple protocol, described by the following type:
>> {noformat}
>> type Test_PDU = record {
>>    header: uint32;
>>    msg_length: uint32;
>>    msg_data: uint32; 
>> } &byteorder=littleendian, &length=msg_length;
>> {noformat}
>> The code generated by BinPAC, when you compile the attached .pac file is wrong.
>> In fact the code generated for the parsing of the message is something like:
>> {noformat}
>> switch ( buffering_state_ ) {
>>    case 0:
>>        ..........
>> 	    t_flow_buffer->NewFrame(8, false);
>>            buffering_state_ = 1;
>>        ..........
>> 	break;
>> 
>>    case 1: {
>>         buffering_state_ = 2;
>> 	  // Checking out-of-bound for "Test_PDU:msg_data"
>> 	  if ( (t_begin_of_data + 8) + (4) > t_end_of_data )
>> 				{
>> 				// Handle out-of-bound condition
>> 				throw ExceptionOutOfBound("Test_PDU:msg_data",
>> 					(8) + (4), 
>> 					(t_end_of_data) - (t_begin_of_data));
>> 				}
>> 	  // Parse "msg_length"
>> 	 msg_length_ = FixByteOrder(byteorder(), *((uint32 const *) ((t_begin_of_data + 4))));
>> 	 t_flow_buffer->GrowFrame(msg_length());
>>    }
>>    break;
>> }
>> {noformat}
>> As you can see at first buffer's length is set to 8, than it will throw an ExceptionOutOfBound because 12>8.
>> I've looked into the issue and i think that the problem  is in the method:
>> bool RecordField::AttemptBoundaryCheck(Output* out_cc, Env* env)
>> (pac_record.cc)
>> In this method the boundary check for the field "msg_length" leads to the boundary check of the field "msg_type", because 
>> quoting the comment on the method: "If my next field can check its boundary, then I don't have to check mine, and it will save me a boundary-check."
>> As a temporary fix i commented out the "optimization" to check the next field in the AttemptBoundaryCheck method.
>> How to fix this issue properly?
> 
> 
> 
> --
> This message was sent by Atlassian JIRA
> (v6.3-OD-06-017#6327)
> _______________________________________________
> bro-dev mailing list
> bro-dev at bro.org
> http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-dev

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 841 bytes
Desc: Message signed with OpenPGP using GPGMail
Url : http://mailman.icsi.berkeley.edu/pipermail/bro-dev/attachments/20140603/c5c5ce33/attachment.bin