Mirror/zeek
1
0
Fork 0
mirror of https://github.com/zeek/zeek.git synced 2025-10-02 14:48:21 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

binpac: Fix signed integer overflow in array bounds checks

Browse source 
Array lengths use signed integer storage, so multiplication of that
by the element size for purpose of bounds checking against available
data may produce a signed integer overlow, which is undefined behavior.
This commit is contained in:
Jon Siwek 2019-06-26 23:31:59 -07:00 • committed by Tim Wojtulewicz
parent 7632c69566
commit ef933c9e76
1 changed files with 22 additions and 9 deletions
Download patch file Download diff file Expand all files Collapse all files

31
tools/binpac/src/pac_array.cc
View file

@ -292,7 +292,7 @@ void ArrayType::GenArrayLength(Output *out_cc, Env *env, const DataPtr& data)
out_cc->println("}"); out_cc->println("}");
out_cc->dec_indent(); out_cc->dec_indent();
string array_size; int element_size;
if ( elemtype_->StaticSize(env) == -1 ) if ( elemtype_->StaticSize(env) == -1 )
{ {
@ -307,7 +307,7 @@ void ArrayType::GenArrayLength(Output *out_cc, Env *env, const DataPtr& data)
// buffer. // buffer.
out_cc->println("// Check array element quantity: %s", out_cc->println("// Check array element quantity: %s",
data_id_str_.c_str()); data_id_str_.c_str());
array_size = strfmt("(%s)", env->RValue(arraylength_var())); element_size = 1;
} }
else else
{ {
@ -315,21 +315,34 @@ void ArrayType::GenArrayLength(Output *out_cc, Env *env, const DataPtr& data)
out_cc->println("// Check bounds for static-size array: %s", out_cc->println("// Check bounds for static-size array: %s",
data_id_str_.c_str()); data_id_str_.c_str());
elemtype_->SetBoundaryChecked(); elemtype_->SetBoundaryChecked();
array_size = strfmt("((%d) * (%s))", element_size = elemtype_->StaticSize(env);
elemtype_->StaticSize(env),
env->RValue(arraylength_var())); if ( element_size == 0 )
{
// If we know we have an array of empty elements, probably
// better to structure the parser as just a single empty
// field to avoid DoS vulnerability of allocating
// arbitrary number of empty records (i.e. cheap for them,
// but costly for us unless we have special optimization
// for this scenario to forgo the usual allocation).
throw Exception(this, "using an array of known-to-be-empty elements is possibly a bad idea");
}
} }
const char* array_ptr_expr = data.ptr_expr(); const char* array_ptr_expr = data.ptr_expr();
string max_elements_available = strfmt("((%s - %s) / %d)",
env->RValue(end_of_data),
array_ptr_expr,
element_size);
out_cc->println("if ( %s + %s > %s || %s + %s < %s )", out_cc->println("if ( %s > %s )",
array_ptr_expr, array_size.c_str(), env->RValue(end_of_data), env->RValue(arraylength_var()),
array_ptr_expr, array_size.c_str(), array_ptr_expr); max_elements_available.c_str());
out_cc->inc_indent(); out_cc->inc_indent();
out_cc->println("throw binpac::ExceptionOutOfBound(\"%s\",", out_cc->println("throw binpac::ExceptionOutOfBound(\"%s\",",
data_id_str_.c_str()); data_id_str_.c_str());
out_cc->println(" %s, (%s) - (%s));", out_cc->println(" %s, (%s) - (%s));",
array_size.c_str(), env->RValue(arraylength_var()),
env->RValue(end_of_data), env->RValue(end_of_data),
array_ptr_expr); array_ptr_expr);
out_cc->dec_indent(); out_cc->dec_indent();