From 8648820497b3e3c0159e4e32befdddad31266a27 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 23 Jan 2015 09:56:59 -0600 Subject: [PATCH] binpac: Fix potential out-of-bounds memory reads in generated code. Field lengths derived from other data in the input could potentially lead to reading from outside the bounds of the input buffer. Reported by John Villamil and Chris Rohlf - Yahoo Paranoids --- tools/binpac/src/pac_dataptr.cc | 7 ++++-- tools/binpac/src/pac_record.cc | 39 +++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/tools/binpac/src/pac_dataptr.cc b/tools/binpac/src/pac_dataptr.cc index 44c497efea..08071b1cf8 100644 --- a/tools/binpac/src/pac_dataptr.cc +++ b/tools/binpac/src/pac_dataptr.cc @@ -41,10 +41,13 @@ void DataPtr::GenBoundaryCheck(Output* out_cc, Env* env, ASSERT(id_); out_cc->println("// Checking out-of-bound for \"%s\"", data_name); - out_cc->println("if ( %s + (%s) > %s )", + out_cc->println("if ( %s + (%s) > %s || %s + (%s) < %s )", ptr_expr(), data_size, - env->RValue(end_of_data)); + env->RValue(end_of_data), + ptr_expr(), + data_size, + ptr_expr()); out_cc->inc_indent(); out_cc->println("{"); diff --git a/tools/binpac/src/pac_record.cc b/tools/binpac/src/pac_record.cc index b474d9c83a..cdffd70ef9 100644 --- a/tools/binpac/src/pac_record.cc +++ b/tools/binpac/src/pac_record.cc @@ -602,10 +602,37 @@ void RecordPaddingField::GenFieldEnd(Output* out_cc, Env* env, const DataPtr& fi switch ( ptype_ ) { case PAD_BY_LENGTH: + out_cc->println("if ( (%s) < 0 ) // check for negative pad length", + expr_->EvalExpr(out_cc, env)); + out_cc->inc_indent(); + out_cc->println("{"); + out_cc->println("throw binpac::ExceptionInvalidStringLength(\"%s\", %s);", + Location(), expr_->EvalExpr(out_cc, env)); + out_cc->println("}"); + out_cc->dec_indent(); + out_cc->println(""); + out_cc->println("const_byteptr const %s = %s + (%s);", env->LValue(end_of_field_dataptr_var), field_begin.ptr_expr(), expr_->EvalExpr(out_cc, env)); + + out_cc->println("// Checking out-of-bound padding for \"%s\"", field_id_str_.c_str()); + out_cc->println("if ( %s > %s || %s < %s )", + env->LValue(end_of_field_dataptr_var), + env->RValue(end_of_data), + env->LValue(end_of_field_dataptr_var), + field_begin.ptr_expr()); + out_cc->inc_indent(); + out_cc->println("{"); + out_cc->println("throw binpac::ExceptionOutOfBound(\"%s\",", field_id_str_.c_str()); + out_cc->println(" (%s), ", + expr_->EvalExpr(out_cc, env)); + out_cc->println(" (%s) - (%s));", + env->RValue(end_of_data), env->LValue(end_of_field_dataptr_var)); + out_cc->println("}"); + out_cc->dec_indent(); + out_cc->println(""); break; case PAD_TO_OFFSET: @@ -628,6 +655,18 @@ void RecordPaddingField::GenFieldEnd(Output* out_cc, Env* env, const DataPtr& fi field_begin.ptr_expr()); out_cc->println("}"); out_cc->dec_indent(); + out_cc->println("if ( %s > %s )", + env->LValue(end_of_field_dataptr_var), + env->RValue(end_of_data)); + out_cc->inc_indent(); + out_cc->println("{"); + out_cc->println("throw binpac::ExceptionOutOfBound(\"%s\",", field_id_str_.c_str()); + out_cc->println(" (%s), ", + expr_->EvalExpr(out_cc, env)); + out_cc->println(" (%s) - (%s));", + env->RValue(end_of_data), env->LValue(end_of_field_dataptr_var)); + out_cc->println("}"); + out_cc->dec_indent(); break; case PAD_TO_NEXT_WORD: