# BinPac generated code: checking out-of-bound too early

**URL:** <https://community.zeek.org/t/binpac-generated-code-checking-out-of-bound-too-early/5625>\
**Category:** Development\
**Tags:** development\
**Created:** [March 6, 2019, 2:14pm UTC](https://community.zeek.org/t/binpac-generated-code-checking-out-of-bound-too-early/5625 "2019-03-06T14:14:19Z")\
**Posts on this page:** 4\
**Page:** 1

<div class="post-metadata">

**Author:** ![Song](https://avatars.discourse-cdn.com/v4/letter/s/ecd19e/32.png) [@Song](https://community.zeek.org/u/Song)\
**Post date:** [March 6, 2019, 2:14pm UTC](https://community.zeek.org/t/binpac-generated-code-checking-out-of-bound-too-early/5625/1 "2019-03-06T14:14:19Z")

</div>

Hi,

I’m writing a BinPac flowunit analyzer, a PDU is like below:

type test\_pdu = record {  
lenAB : uint32; # length of rest of data  
lenA : uint16; # length of dataA  
dataA : bytestring &length = lenA;  
dataB : bytestring &length = (lenAB - 2 - lenA);  
} &byteorder=bigendian &length=(lenAB + 4);

There are 2 problems:

1. binpac failed to compile (cannot handle incremental input) if I remove &length=(lenAB - 2 -lenA), although the overall length of the PDU can be calculated using the 4 field length

2. the generated parser seems to check out-of-bound of lenA field too early:

1577 bool test\_pdu::ParseBuffer(flow\_buffer\_t t\_flow\_buffer)  
1578 {  
1579 bool t\_val\_parsing\_complete;  
1580 t\_val\_parsing\_complete = false;  
1581 const\_byteptr t\_begin\_of\_data = t\_flow\_buffer-\>begin();  
1582 const\_byteptr t\_end\_of\_data = t\_flow\_buffer-\>end();  
1583 switch ( buffering\_state\_ )  
1584 {  
1585 case 0:  
1586 if ( buffering\_state\_ == 0 )  
1587 {  
1588 t\_flow\_buffer-\>NewFrame(4, false);  
1589 buffering\_state\_ = 1;  
1590 }  
1591 buffering\_state\_ = 1;  
1592 break;  
1593 case 1:  
1594 {  
1595 buffering\_state\_ = 2;  
1596 // Checking out-of-bound for “test\_pdu:lenA”  
1597 if ( (t\_begin\_of\_data + 4) + (2) \> t\_end\_of\_data || (t\_begin\_of\_data + 4) + (2) \< (t\_begin\_of\_data + 4) )  
1598 {  
1599 // Handle out-of-bound condition  
1600 throw binpac::ExceptionOutOfBound(“test\_pdu:lenA”,  
1601 (4) + (2),  
1602 (t\_end\_of\_data) - (t\_begin\_of\_data));  
1603 }  
1604 // Parse “lenAB”  
1605 lenAB\_ = FixByteOrder(byteorder(), \*((uint32 const \*) (t\_begin\_of\_data)));  
1606 // Evaluate ‘let’ and ‘withinput’ fields  
1607 t\_flow\_buffer-\>GrowFrame( ( lenAB() + 4 ) );  
1608 }  
1609 break;

Since we only make a new frame of length 4 in line #1588 (the flow buffer will not grow to full size until line #1607), the test in line #1597 will be evaluated to true and the parsing will fail.

What did I missed? Thanks in advance.

Best regards,  
Song

---

<div class="post-metadata">

**Author:** ![Vlad\_Grigorescu](https://avatars.discourse-cdn.com/v4/letter/v/a3d4f5/32.png) [@Vlad\_Grigorescu](https://community.zeek.org/u/Vlad_Grigorescu)\
**Post date:** [March 6, 2019, 3:19pm UTC](https://community.zeek.org/t/binpac-generated-code-checking-out-of-bound-too-early/5625/2 "2019-03-06T15:19:35Z")

</div>

I’d try reworking this, so that you have lenAB, and then another record for the rest of the data, and just setting the length of that record to lenAB. Does that work?

—Vlad

---

<div class="post-metadata">

**Author:** ![Song](https://avatars.discourse-cdn.com/v4/letter/s/ecd19e/32.png) [@Song](https://community.zeek.org/u/Song)\
**Post date:** [March 7, 2019, 3:19am UTC](https://community.zeek.org/t/binpac-generated-code-checking-out-of-bound-too-early/5625/3 "2019-03-07T03:19:22Z")

</div>

Thanks Vlad.

I tried as you suggestd and it works:

type test\_pdu = record {  
lenAB : uint32;  
pduAB : test\_pdu\_ab(lenAB);  
} &byteorder=bigendian &length=(lenAB + 4); # fail to compile without &length

type test\_pdu\_ab(len: uint32) = record {  
lenA : uint16;  
dataA : bytestring &length = lenA;  
dataB : bytestring &length = (len - 2 - lenA);  
} &byteorder=bigendian; # do not need &length here

The result is correct:

1577 bool test\_pdu::ParseBuffer(flow\_buffer\_t t\_flow\_buffer)  
1578 {  
1579 bool t\_val\_parsing\_complete;  
1580 t\_val\_parsing\_complete = false;  
1581 const\_byteptr t\_begin\_of\_data = t\_flow\_buffer-\>begin();  
1582 const\_byteptr t\_end\_of\_data = t\_flow\_buffer-\>end();  
1583 switch ( buffering\_state\_ )  
1584 {  
1585 case 0:  
1586 if ( buffering\_state\_ == 0 )  
1587 {  
1588 t\_flow\_buffer-\>NewFrame(4, false);  
1589 buffering\_state\_ = 1;  
1590 }  
1591 buffering\_state\_ = 1;  
1592 break;  
1593 case 1:  
1594 {  
1595 buffering\_state\_ = 2;  
1596 // Checking out-of-bound for “test\_pdu:lenAB”  
1597 if ( t\_begin\_of\_data + (4) \> t\_end\_of\_data || t\_begin\_of\_data + (4) \< t\_begin\_of\_data )  
1598 {  
1599 // Handle out-of-bound condition  
1600 throw binpac::ExceptionOutOfBound(“test\_pdu:lenAB”,  
1601 (0) + (4),  
1602 (t\_end\_of\_data) - (t\_begin\_of\_data));  
1603 }  
1604 // Parse “lenAB”  
1605 lenAB\_ = FixByteOrder(byteorder(), \*((uint32 const \*) (t\_begin\_of\_data)));  
1606 // Evaluate ‘let’ and ‘withinput’ fields  
1607 t\_flow\_buffer-\>GrowFrame( ( lenAB() + 4 ) );  
1608 }  
1609 break;  
1610 case 2:  
1611 BINPAC\_ASSERT(t\_flow\_buffer-\>ready());  
1612 if ( t\_flow\_buffer-\>ready() )  
1613 {  
1614  
1615 // Parse “pduAB”  
1616 pduAB\_ = new test\_pdu\_ab(lenAB());  
1617 int t\_pduAB\_\_size;  
1618 t\_pduAB\_\_size = pduAB\_-\>Parse((t\_begin\_of\_data + 4), t\_end\_of\_data);  
…  
…  
1649  
1650 int test\_pdu\_ab::Parse(const\_byteptr const t\_begin\_of\_data, const\_byteptr const t\_end\_of\_data)  
1651 {  
1652 // Checking out-of-bound for “test\_pdu\_ab:lenA”  
1653 if ( t\_begin\_of\_data + (2) \> t\_end\_of\_data || t\_begin\_of\_data + (2) \< t\_begin\_of\_data )  
1654 {  
1655 // Handle out-of-bound condition  
1656 throw binpac::ExceptionOutOfBound(“test\_pdu\_ab:lenA”,  
1657 (0) + (2),  
1658 (t\_end\_of\_data) - (t\_begin\_of\_data));  
1659 }  
1660 // Parse “lenA”  
1661 lenA\_ = FixByteOrder(byteorder(), \*((uint16 const \*) (t\_begin\_of\_data)));  
1662 // Evaluate ‘let’ and ‘withinput’ fields  
1663  
1664 // Parse “dataA”  
1665 int t\_dataA\_\_size;  
1666 t\_dataA\_\_size = lenA();  
1667 // Checking out-of-bound for “test\_pdu\_ab:dataA”  
1668 if ( (t\_begin\_of\_data + 2) + (t\_dataA\_\_size) \> t\_end\_of\_data || (t\_begin\_of\_data + 2) + (t\_dataA\_\_size) \< (t\_begin\_of\_data + 2) )  
1669 {

Does it necessary to do those out-of-bound checks like in line #1597 #1653 #1668? I think we have already got filled frame with correct size when these checks are performed.

Thanks again and best regards,  
Song

------------------ Original ------------------

---

<div class="post-metadata">

**Author:** ![system](https://canada1.discourse-cdn.com/flex011/uploads/zeek/original/1X/f09d732bc2cc7c7cc7e35db67cf4e1d5233ce7a7.png) [@system](https://community.zeek.org/u/system)\
**Post date:** [May 6, 2022, 3:46pm UTC](https://community.zeek.org/t/binpac-generated-code-checking-out-of-bound-too-early/5625/4 "2022-05-06T15:46:22Z")

</div>