From cb6f6467c7f93da79c755479cc8eba40f6c75ae3 Mon Sep 17 00:00:00 2001 From: Jon Crussell Date: Sat, 9 Nov 2013 18:04:36 -0800 Subject: [PATCH 1/2] Fixed Segmentation fault in SQLite Writer. Segmentation fault caused by accessing fields with pos which is one-based for setting SQLite field values. Fix is to simply subtract one from pos. Discovered when trying to store HTTP traffic to a SQLite database with the following Bro script: event bro_init() { local filter: Log::Filter = [ $name = "sqlite", $path = "http", $config = table(["tablename"] = "http_logs"), $writer = Log::WRITER_SQLITE ]; Log::add_filter(HTTP::LOG, filter); } --- src/logging/writers/SQLite.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/logging/writers/SQLite.cc b/src/logging/writers/SQLite.cc index 37e3134659..46d1f17130 100644 --- a/src/logging/writers/SQLite.cc +++ b/src/logging/writers/SQLite.cc @@ -308,7 +308,7 @@ int SQLite::AddParams(Value* val, int pos) if ( j > 0 ) desc.AddRaw(set_separator); - io->Describe(&desc, val->val.set_val.vals[j], fields[pos]->name); + io->Describe(&desc, val->val.set_val.vals[j], fields[pos-1]->name); } desc.RemoveEscapeSequence(set_separator); @@ -330,7 +330,7 @@ int SQLite::AddParams(Value* val, int pos) if ( j > 0 ) desc.AddRaw(set_separator); - io->Describe(&desc, val->val.vector_val.vals[j], fields[pos]->name); + io->Describe(&desc, val->val.vector_val.vals[j], fields[pos-1]->name); } desc.RemoveEscapeSequence(set_separator); From 81d0def32706f0a6b387876f651bd244b6c90036 Mon Sep 17 00:00:00 2001 From: Bernhard Amann Date: Sun, 10 Nov 2013 22:04:16 -0800 Subject: [PATCH 2/2] Add minimal testcase for sqlite writer crash. The writer did not work with a non-empty set or vector as the last element in the logged column. --- .../ssh.select | 1 + .../base/frameworks/logging/sqlite/set.bro | 50 +++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 testing/btest/Baseline/scripts.base.frameworks.logging.sqlite.set/ssh.select create mode 100644 testing/btest/scripts/base/frameworks/logging/sqlite/set.bro diff --git a/testing/btest/Baseline/scripts.base.frameworks.logging.sqlite.set/ssh.select b/testing/btest/Baseline/scripts.base.frameworks.logging.sqlite.set/ssh.select new file mode 100644 index 0000000000..71bf36b865 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.frameworks.logging.sqlite.set/ssh.select @@ -0,0 +1 @@ +CC,AA,BB diff --git a/testing/btest/scripts/base/frameworks/logging/sqlite/set.bro b/testing/btest/scripts/base/frameworks/logging/sqlite/set.bro new file mode 100644 index 0000000000..0da88d5e38 --- /dev/null +++ b/testing/btest/scripts/base/frameworks/logging/sqlite/set.bro @@ -0,0 +1,50 @@ +# +# Check if set wors in last position (the describe call in sqlite.cc has a good +# chance of being off by one if someone changes it). +# +# @TEST-REQUIRES: which sqlite3 +# @TEST-REQUIRES: has-writer SQLite +# @TEST-GROUP: sqlite +# +# @TEST-EXEC: bro -b %INPUT +# @TEST-EXEC: sqlite3 ssh.sqlite 'select * from ssh' > ssh.select +# @TEST-EXEC: btest-diff ssh.select +# +# Testing all possible types. + +redef LogSQLite::unset_field = "(unset)"; + +module SSH; + +export { + redef enum Log::ID += { LOG }; + + type Log: record { + ss: set[string]; + } &log; +} + +function foo(i : count) : string + { + if ( i > 0 ) + return "Foo"; + else + return "Bar"; + } + +event bro_init() +{ + Log::create_stream(SSH::LOG, [$columns=Log]); + Log::remove_filter(SSH::LOG, "default"); + + local filter: Log::Filter = [$name="sqlite", $path="ssh", $writer=Log::WRITER_SQLITE]; + Log::add_filter(SSH::LOG, filter); + + local empty_set: set[string]; + local empty_vector: vector of string; + + Log::write(SSH::LOG, [ + $ss=set("AA", "BB", "CC") + ]); +} +