From 4318d5ab9e65f8e5bcd8b2c7df0b18feb2980e19 Mon Sep 17 00:00:00 2001 From: Robin Sommer Date: Fri, 14 Jun 2024 09:55:39 +0200 Subject: [PATCH] Spicy: Disallow repeating replacements of the same analyzer. We now reject EVT files that attempt to replace the same built-in analyzer multiple times as doing so would be ill-defined and not very intuitive in what exactly it means. Closes #3783. --- src/spicy/manager.cc | 33 ++++++++++++--- .../spicy.replaces-conflicts-2/output | 2 + .../spicy.replaces-conflicts-3/output | 2 + .../Baseline/spicy.replaces-conflicts/output | 2 + testing/btest/spicy/replaces-conflicts.evt | 42 +++++++++++++++++++ 5 files changed, 75 insertions(+), 6 deletions(-) create mode 100644 testing/btest/Baseline/spicy.replaces-conflicts-2/output create mode 100644 testing/btest/Baseline/spicy.replaces-conflicts-3/output create mode 100644 testing/btest/Baseline/spicy.replaces-conflicts/output create mode 100644 testing/btest/spicy/replaces-conflicts.evt diff --git a/src/spicy/manager.cc b/src/spicy/manager.cc index 9ad40b10ad..1a9420e22a 100644 --- a/src/spicy/manager.cc +++ b/src/spicy/manager.cc @@ -897,14 +897,21 @@ void Manager::disableReplacedAnalyzers() { if ( file_mgr->Lookup(replaces, false) || packet_mgr->Lookup(replaces, false) ) reporter->FatalError("cannot replace '%s' analyzer with a protocol analyzer", replaces); - auto tag = analyzer_mgr->GetAnalyzerTag(replaces); - if ( ! tag ) { + auto component = analyzer_mgr->Lookup(replaces, false); + if ( ! component ) { SPICY_DEBUG(hilti::rt::fmt("%s is supposed to replace protocol analyzer %s, but that does not exist", info.name_analyzer, replaces)); continue; } + auto tag = component->Tag(); + if ( analyzer_mgr->HasComponentMapping(tag) ) + reporter->FatalError( + "%s: protocol analyzer %s is already mapped to a different analyzer; cannot replace an analyzer " + "multiple times", + info.name_analyzer.c_str(), component->Name().c_str()); + SPICY_DEBUG(hilti::rt::fmt("%s replaces existing protocol analyzer %s", info.name_analyzer, replaces)); info.replaces = tag; analyzer_mgr->DisableAnalyzer(tag); @@ -928,10 +935,17 @@ void Manager::disableReplacedAnalyzers() { continue; } + auto tag = component->Tag(); + if ( file_mgr->HasComponentMapping(tag) ) + reporter->FatalError( + "%s: file analyzer %s is already mapped to a different analyzer; cannot replace an analyzer multiple " + "times", + info.name_analyzer.c_str(), component->Name().c_str()); + SPICY_DEBUG(hilti::rt::fmt("%s replaces existing file analyzer %s", info.name_analyzer, replaces)); - info.replaces = component->Tag(); + info.replaces = tag; component->SetEnabled(false); - file_mgr->AddComponentMapping(component->Tag(), info.tag); + file_mgr->AddComponentMapping(tag, info.tag); } for ( auto& info : _packet_analyzers_by_type ) { @@ -948,10 +962,17 @@ void Manager::disableReplacedAnalyzers() { continue; } + auto tag = component->Tag(); + if ( packet_mgr->HasComponentMapping(tag) ) + reporter->FatalError( + "%s: packet analyzer %s is already mapped to a different analyzer; cannot replace an analyzer multiple " + "times", + info.name_analyzer.c_str(), component->Name().c_str()); + SPICY_DEBUG(hilti::rt::fmt("%s replaces existing packet analyzer %s", info.name_analyzer, replaces)); - info.replaces = component->Tag(); + info.replaces = tag; component->SetEnabled(false); - packet_mgr->AddComponentMapping(component->Tag(), info.tag); + packet_mgr->AddComponentMapping(tag, info.tag); } } diff --git a/testing/btest/Baseline/spicy.replaces-conflicts-2/output b/testing/btest/Baseline/spicy.replaces-conflicts-2/output new file mode 100644 index 0000000000..b4f454e6e7 --- /dev/null +++ b/testing/btest/Baseline/spicy.replaces-conflicts-2/output @@ -0,0 +1,2 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +fatal error: spicy::SSH_2: file analyzer MD5 is already mapped to a different analyzer; cannot replace an analyzer multiple times diff --git a/testing/btest/Baseline/spicy.replaces-conflicts-3/output b/testing/btest/Baseline/spicy.replaces-conflicts-3/output new file mode 100644 index 0000000000..0733458438 --- /dev/null +++ b/testing/btest/Baseline/spicy.replaces-conflicts-3/output @@ -0,0 +1,2 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +fatal error: spicy::SSH_2: packet analyzer Ethernet is already mapped to a different analyzer; cannot replace an analyzer multiple times diff --git a/testing/btest/Baseline/spicy.replaces-conflicts/output b/testing/btest/Baseline/spicy.replaces-conflicts/output new file mode 100644 index 0000000000..8cf95b5195 --- /dev/null +++ b/testing/btest/Baseline/spicy.replaces-conflicts/output @@ -0,0 +1,2 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +fatal error: redefinition of protocol analyzer spicy::SSH_1 diff --git a/testing/btest/spicy/replaces-conflicts.evt b/testing/btest/spicy/replaces-conflicts.evt new file mode 100644 index 0000000000..dbca6d637e --- /dev/null +++ b/testing/btest/spicy/replaces-conflicts.evt @@ -0,0 +1,42 @@ +# @TEST-REQUIRES: have-spicy +# +# @TEST-EXEC: spicyz -d -o ssh.hlto ssh.spicy %INPUT +# @TEST-EXEC-FAIL: zeek ssh.hlto >output 2>&1 +# @TEST-EXEC: btest-diff output + +# @TEST-START-FILE ssh.spicy +module SSH; + +import zeek; + +public type Banner = unit {}; +# @TEST-END-FILE + +protocol analyzer spicy::SSH_1 over TCP: + parse with SSH::Banner, + replaces SSH; + +protocol analyzer spicy::SSH_1 over UDP: + parse with SSH::Banner, + replaces SSH; + +# @TEST-START-NEXT + +file analyzer spicy::SSH_1: + parse with SSH::Banner, + replaces MD5; + +file analyzer spicy::SSH_2: + parse with SSH::Banner, + replaces MD5; + +# @TEST-START-NEXT + +packet analyzer spicy::SSH_1: + parse with SSH::Banner, + replaces Ethernet; + +packet analyzer spicy::SSH_2: + parse with SSH::Banner, + replaces Ethernet; +