From e616554ab8a475d3b33463353ad0fa9a8645d0d9 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Tue, 10 Jun 2014 13:38:32 -0500 Subject: [PATCH] Fix use-after-free in some cases of reassigning a table index. Specifically observed when redef'ing the same index of a table that uses subnets as indices, though the bug seems like it applies more generally to anytime TableVal::Assign is provided with just the HashKey parameter and not the index Val. Addresses BIT-1202. --- src/Val.cc | 11 ++++++++--- .../language.redef-same-prefixtable-idx/out | 4 ++++ .../language/redef-same-prefixtable-idx.bro | 17 +++++++++++++++++ 3 files changed, 29 insertions(+), 3 deletions(-) create mode 100644 testing/btest/Baseline/language.redef-same-prefixtable-idx/out create mode 100644 testing/btest/language/redef-same-prefixtable-idx.bro diff --git a/src/Val.cc b/src/Val.cc index 2d75680182..6503cd486f 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -1471,13 +1471,19 @@ int TableVal::Assign(Val* index, HashKey* k, Val* new_val, Opcode op) } TableEntryVal* new_entry_val = new TableEntryVal(new_val); + HashKey k_copy(k->Key(), k->Size(), k->Hash()); TableEntryVal* old_entry_val = AsNonConstTable()->Insert(k, new_entry_val); + // If the dictionary index already existed, the insert may free up the + // memory allocated to the key bytes, so have to assume k is invalid + // from here on out. + delete k; + k = 0; if ( subnets ) { if ( ! index ) { - Val* v = RecoverIndex(k); + Val* v = RecoverIndex(&k_copy); subnets->Insert(v, new_entry_val); Unref(v); } @@ -1489,7 +1495,7 @@ int TableVal::Assign(Val* index, HashKey* k, Val* new_val, Opcode op) { Val* rec_index = 0; if ( ! index ) - index = rec_index = RecoverIndex(k); + index = rec_index = RecoverIndex(&k_copy); if ( new_val ) { @@ -1547,7 +1553,6 @@ int TableVal::Assign(Val* index, HashKey* k, Val* new_val, Opcode op) if ( old_entry_val && attrs && attrs->FindAttr(ATTR_EXPIRE_CREATE) ) new_entry_val->SetExpireAccess(old_entry_val->ExpireAccessTime()); - delete k; if ( old_entry_val ) { old_entry_val->Unref(); diff --git a/testing/btest/Baseline/language.redef-same-prefixtable-idx/out b/testing/btest/Baseline/language.redef-same-prefixtable-idx/out new file mode 100644 index 0000000000..aa6f21177a --- /dev/null +++ b/testing/btest/Baseline/language.redef-same-prefixtable-idx/out @@ -0,0 +1,4 @@ +{ +[3.0.0.0/8] = 2.0.0.0/8 +} +2.0.0.0/8 diff --git a/testing/btest/language/redef-same-prefixtable-idx.bro b/testing/btest/language/redef-same-prefixtable-idx.bro new file mode 100644 index 0000000000..13cf27cc0f --- /dev/null +++ b/testing/btest/language/redef-same-prefixtable-idx.bro @@ -0,0 +1,17 @@ +# @TEST-EXEC: bro -b %INPUT >out +# @TEST-EXEC: btest-diff out + +const my_table: table[subnet] of subnet &redef; + +redef my_table[3.0.0.0/8] = 1.0.0.0/8; +redef my_table[3.0.0.0/8] = 2.0.0.0/8; + +# The above is basically a shorthand for: +# redef my_table += { [3.0.0.0/8] = 1.0.0.0/8 }; +# redef my_table += { [3.0.0.0/8] = 2.0.0.0/8 }; + +event bro_init() + { + print my_table; + print my_table[3.0.0.0/8]; + }