From a9f14bf5031081262e184b8c5e3d52635ae4553f Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 23 Aug 2019 07:22:18 -0400 Subject: [PATCH 1/2] GH-541: fix handling of NTLM AV Pair sequences Empty AV Pair sequences or AV Pair sequences that lack a terminator could cause accesses past the end of the parsed vector. --- src/analyzer/protocol/ntlm/ntlm-analyzer.pac | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/analyzer/protocol/ntlm/ntlm-analyzer.pac b/src/analyzer/protocol/ntlm/ntlm-analyzer.pac index 0f0d842570..3d2008808b 100644 --- a/src/analyzer/protocol/ntlm/ntlm-analyzer.pac +++ b/src/analyzer/protocol/ntlm/ntlm-analyzer.pac @@ -24,13 +24,26 @@ refine connection NTLM_Conn += { return result; %} - function build_av_record(val: NTLM_AV_Pair_Sequence): BroVal + function build_av_record(val: NTLM_AV_Pair_Sequence, len: uint16): BroVal %{ RecordVal* result = new RecordVal(BifType::Record::NTLM::AVs); - for ( uint i = 0; ${val.pairs[i].id} != 0; i++ ) + for ( uint i = 0; ; i++ ) { + if ( i >= ${val.pairs}->size() ) + { + if ( len != 0 ) + // According to spec, the TargetInfo MUST be a sequence of + // AV_PAIRs and terminated by the null AV_PAIR when the + // TargetInfoLen is non-zero, so this is in violation. + bro_analyzer()->ProtocolViolation("NTLM AV Pair loop underflow"); + + return result; + } + switch ( ${val.pairs[i].id} ) { + case 0: + return result; case 1: result->Assign(0, utf16_bytestring_to_utf8_val(bro_analyzer()->Conn(), ${val.pairs[i].nb_computer_name.data})); break; @@ -131,7 +144,7 @@ refine connection NTLM_Conn += { result->Assign(2, build_version_record(${val.version})); if ( ${val}->has_target_info() ) - result->Assign(3, build_av_record(${val.target_info})); + result->Assign(3, build_av_record(${val.target_info}, ${val.target_info_fields.length})); BifEvent::generate_ntlm_challenge(bro_analyzer(), bro_analyzer()->Conn(), From b95476748884f773b7b3bb9b7aecf5eacbc72a48 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Mon, 26 Aug 2019 10:28:46 -0700 Subject: [PATCH 2/2] GH-541: add test cases for NTLM AV Pair sequence handling --- .../dpd.log | 10 ++++++++++ .../ntlm.log | 10 ++++++++++ .../dpd.log | 10 ++++++++++ .../ntlm.log | 10 ++++++++++ .../Traces/dce-rpc/ntlm-empty-av-sequence.pcap | Bin 0 -> 788 bytes .../dce-rpc/ntlm-unterminated-av-sequence.pcap | Bin 0 -> 820 bytes .../protocols/dce-rpc/ntlm-empty-av-pair-seq.zeek | 8 ++++++++ .../dce-rpc/ntlm-unterminated-av-pair-seq.zeek | 8 ++++++++ 8 files changed, 56 insertions(+) create mode 100644 testing/btest/Baseline/scripts.base.protocols.dce-rpc.ntlm-empty-av-pair-seq/dpd.log create mode 100644 testing/btest/Baseline/scripts.base.protocols.dce-rpc.ntlm-empty-av-pair-seq/ntlm.log create mode 100644 testing/btest/Baseline/scripts.base.protocols.dce-rpc.ntlm-unterminated-av-pair-seq/dpd.log create mode 100644 testing/btest/Baseline/scripts.base.protocols.dce-rpc.ntlm-unterminated-av-pair-seq/ntlm.log create mode 100644 testing/btest/Traces/dce-rpc/ntlm-empty-av-sequence.pcap create mode 100644 testing/btest/Traces/dce-rpc/ntlm-unterminated-av-sequence.pcap create mode 100644 testing/btest/scripts/base/protocols/dce-rpc/ntlm-empty-av-pair-seq.zeek create mode 100644 testing/btest/scripts/base/protocols/dce-rpc/ntlm-unterminated-av-pair-seq.zeek diff --git a/testing/btest/Baseline/scripts.base.protocols.dce-rpc.ntlm-empty-av-pair-seq/dpd.log b/testing/btest/Baseline/scripts.base.protocols.dce-rpc.ntlm-empty-av-pair-seq/dpd.log new file mode 100644 index 0000000000..e0ac743b18 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.dce-rpc.ntlm-empty-av-pair-seq/dpd.log @@ -0,0 +1,10 @@ +#separator \x09 +#set_separator , +#empty_field (empty) +#unset_field - +#path dpd +#open 2019-08-26-17-26-38 +#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p proto analyzer failure_reason +#types time string addr port addr port enum string string +1056991898.901892 CHhAvVGS1DHFjwGM9 192.168.0.173 1068 192.168.0.2 4997 tcp NTLM NTLM AV Pair loop underflow +#close 2019-08-26-17-26-38 diff --git a/testing/btest/Baseline/scripts.base.protocols.dce-rpc.ntlm-empty-av-pair-seq/ntlm.log b/testing/btest/Baseline/scripts.base.protocols.dce-rpc.ntlm-empty-av-pair-seq/ntlm.log new file mode 100644 index 0000000000..4af05b67d0 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.dce-rpc.ntlm-empty-av-pair-seq/ntlm.log @@ -0,0 +1,10 @@ +#separator \x09 +#set_separator , +#empty_field (empty) +#unset_field - +#path ntlm +#open 2019-08-26-17-26-38 +#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p username hostname domainname server_nb_computer_name server_dns_computer_name server_tree_name success +#types time string addr port addr port string string string string string string bool +1056991898.900518 CHhAvVGS1DHFjwGM9 192.168.0.173 1068 192.168.0.2 4997 - - - - - - - +#close 2019-08-26-17-26-38 diff --git a/testing/btest/Baseline/scripts.base.protocols.dce-rpc.ntlm-unterminated-av-pair-seq/dpd.log b/testing/btest/Baseline/scripts.base.protocols.dce-rpc.ntlm-unterminated-av-pair-seq/dpd.log new file mode 100644 index 0000000000..206c49fb8f --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.dce-rpc.ntlm-unterminated-av-pair-seq/dpd.log @@ -0,0 +1,10 @@ +#separator \x09 +#set_separator , +#empty_field (empty) +#unset_field - +#path dpd +#open 2019-08-26-17-26-39 +#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p proto analyzer failure_reason +#types time string addr port addr port enum string string +1056991898.901892 CHhAvVGS1DHFjwGM9 192.168.0.173 1068 192.168.0.2 4997 tcp NTLM NTLM AV Pair loop underflow +#close 2019-08-26-17-26-39 diff --git a/testing/btest/Baseline/scripts.base.protocols.dce-rpc.ntlm-unterminated-av-pair-seq/ntlm.log b/testing/btest/Baseline/scripts.base.protocols.dce-rpc.ntlm-unterminated-av-pair-seq/ntlm.log new file mode 100644 index 0000000000..7626e6f0ea --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.dce-rpc.ntlm-unterminated-av-pair-seq/ntlm.log @@ -0,0 +1,10 @@ +#separator \x09 +#set_separator , +#empty_field (empty) +#unset_field - +#path ntlm +#open 2019-08-26-17-26-39 +#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p username hostname domainname server_nb_computer_name server_dns_computer_name server_tree_name success +#types time string addr port addr port string string string string string string bool +1056991898.900518 CHhAvVGS1DHFjwGM9 192.168.0.173 1068 192.168.0.2 4997 - - - SATURN - - - +#close 2019-08-26-17-26-39 diff --git a/testing/btest/Traces/dce-rpc/ntlm-empty-av-sequence.pcap b/testing/btest/Traces/dce-rpc/ntlm-empty-av-sequence.pcap new file mode 100644 index 0000000000000000000000000000000000000000..2d7e7631f904e027ee326ba163fd1c3f05088ead GIT binary patch literal 788 zcmd<$<>iuLU|{gI(UxKa(*L0VBnIM(KCTHI#uA7cl+$pv21n6lC4P z$jHJ3F&AWh3`iOXK<2Lpnr{PSgD}Koh%I3Ab@Fe6%;)$BG8<%va4U<>)6^#_Ks(A4 zgp@4Y3?b%2%tSX^M;ys)bhm4;d4b*DhUWHip!uo+0>3`Anu44J0wDK-%)SBQ00GF| zY7(oYwgcH9j1ev?*}TDKvtTt_;@1avV+ICR25x2npyoyfLk3{rFkBG3Al8_(gb5TY zptzBUI`cKfNlKt6l7m58zeUXgD9RGs(e+y9q~QD)KrsQ21du*15YfiP;1}ZK8yp+} zj#c(e91GZhZq^1$aRZ?OkaqNO_4jiOa?yrOXWEtenKY1Hy3k0YzsDM+BGlL(4BZDu4Cqpn$f)SXiee+W?(=t<26p|_x R!ZV9fT~jhkKzcxa1pwCPsKWpN literal 0 HcmV?d00001 diff --git a/testing/btest/Traces/dce-rpc/ntlm-unterminated-av-sequence.pcap b/testing/btest/Traces/dce-rpc/ntlm-unterminated-av-sequence.pcap new file mode 100644 index 0000000000000000000000000000000000000000..55a9819f3c2a4e82d624e64c79e8ca10f8cd6516 GIT binary patch literal 820 zcmd<$<>iuLU|{gI(UxKa(*L0VBnIM(KCTHI#uA7cl+$pv21n6lC4P z$jHJ3F&AWh3`iOXK<2Lpnr{PSgD}Koh%I3Ab@Fe6%;)$BG8<%va4U<>)6^#_Ks(A4 zgp@4Y3?b%2%tSX^M;ys)bhm4;d4b*DhUWHip!uo+0>3`Anu44J0wDK-%)SBQ00GF| zY7(oYwgcH9j1ev?*}TDKvtTt_;@1avV+ICR25x2npyoyfLk3{rFkBG3Al8_(gb5TY zptzBUI`cKfNlKt6l7m58zeUXgD9RGs(e+y9q~QD)KrsQ21du*15YfiP;1}ZK8yp+} zj#c(e91GZhZq^1$aRZ?OkaqNO_4jiOa?yq)_E5C=PzA=vj{ph9?Yd7v{^9}qYXLCakp0EVU}9-$&cF|~j3~c=^z#6r z0gz^G>QvZ}Wsp1n