Mirror/zeek
1
0
Fork 0
mirror of https://github.com/zeek/zeek.git synced 2025-10-16 13:38:19 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

binpac: Fix incorrect boundary checks in flowbuffer frame length parsing

Browse source 
Incremental flowbuffer parsing sought to first parse the "minimum header
length" required to get the full frame length, possibly from a record
field, but generating the logic to parse that field could greedily
bundle in additional boundary-checks for all subsequent fields of
known-size.

E.g. for flowunit parsing of this:

    type HDR = record {
        version:    uint8;
        reserved:   uint8;
        len:        uint16;
    } &byteorder=bigendian;

    type FOO_PDU(is_orig: bool) = record {
        hdr:        HDR;
        plen:       uint8;
        ptype:      uint8;
        something:  bytestring &restofdata;
    } &byteorder=bigendian, &length=hdr.len;

The flowbuffer was correctly seeking to buffer 4 bytes and parse the
"hdr.len" field, but the generated parsing logic for "hdr.len" included
a boundary check all the way up to include "plen" and "ptype".

This causes out-of-bounds exceptions to be thrown for inputs that should
actually be possible to incrementally parse via flowbuffer.
This commit is contained in:
Jon Siwek 2020-03-19 21:33:28 -07:00 • committed by Tim Wojtulewicz
parent 3aad9c74c3
commit db7c3d7c5c
1 changed files with 7 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files

8
tools/binpac/src/pac_record.cc
View file

@ -431,7 +431,13 @@ void RecordDataField::GenParseCode(Output* out_cc, Env* env)
if ( ! record_type()->incremental_parsing() )
{
data = getFieldBegin(out_cc, env);
AttemptBoundaryCheck(out_cc, env);
Expr* len_expr = record_type()->attr_length_expr();
int len;
if ( ! record_type()->buffer_input() ||
(len_expr && len_expr->ConstFold(env, &len)) )
AttemptBoundaryCheck(out_cc, env);
}
out_cc->println("// Parse \"%s\"", id_->Name());