From db7c3d7c5c54c878a5a055c67117220bbefccf81 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Thu, 19 Mar 2020 21:33:28 -0700 Subject: [PATCH] binpac: Fix incorrect boundary checks in flowbuffer frame length parsing 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. --- tools/binpac/src/pac_record.cc | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/binpac/src/pac_record.cc b/tools/binpac/src/pac_record.cc index 813f201b8b..50f850fba5 100644 --- a/tools/binpac/src/pac_record.cc +++ b/tools/binpac/src/pac_record.cc @@ -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());