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

[grigorescu (JIRA)]

     [ 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)