From 4015beb732bcacac1debfc00366c777f02a9855a Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Wed, 24 Mar 2021 13:31:36 -0700 Subject: [PATCH] Fix crash in Analyzer::ForwardPacket due to recursive analyzer calls. The change in 44f558df7b5a85bae40945de653bcb2448e0a7f4 that made analyzer_list a std::vector instead of a std::list doesn't take into account that in some cases an analyzer may chain back into itself, such as with UDP-in-UDP tunnels. In these cases, the second call to ForwardPacket may cause iterator invalidation, leading to a crash. --- src/analyzer/Analyzer.h | 7 ++++++- .../core.tunnels.teredo-udp-in-udp/tunnel.log | 12 ++++++++++++ .../btest/Traces/tunnels/teredo-udp-in-udp.pcap | Bin 0 -> 175 bytes testing/btest/core/tunnels/teredo-udp-in-udp.zeek | 4 ++++ 4 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 testing/btest/Baseline/core.tunnels.teredo-udp-in-udp/tunnel.log create mode 100644 testing/btest/Traces/tunnels/teredo-udp-in-udp.pcap create mode 100644 testing/btest/core/tunnels/teredo-udp-in-udp.zeek diff --git a/src/analyzer/Analyzer.h b/src/analyzer/Analyzer.h index 8de42eeff7..5999063fe6 100644 --- a/src/analyzer/Analyzer.h +++ b/src/analyzer/Analyzer.h @@ -38,7 +38,12 @@ class AnalyzerTimer; class SupportAnalyzer; class OutputHandler; -using analyzer_list = std::vector; +// This needs to remain a std::list because of the usage of iterators in the +// Analyzer::Forward methods. These methods have the chance to loop back +// into the same analyzer in the case of tunnels. If the recursive call adds +// to the children list, it can invalidate iterators in the outer call, +// causing a crash. +using analyzer_list = std::list; typedef uint32_t ID; typedef void (Analyzer::*analyzer_timer_func)(double t); diff --git a/testing/btest/Baseline/core.tunnels.teredo-udp-in-udp/tunnel.log b/testing/btest/Baseline/core.tunnels.teredo-udp-in-udp/tunnel.log new file mode 100644 index 0000000000..e33e982a23 --- /dev/null +++ b/testing/btest/Baseline/core.tunnels.teredo-udp-in-udp/tunnel.log @@ -0,0 +1,12 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +#separator \x09 +#set_separator , +#empty_field (empty) +#unset_field - +#path tunnel +#open XXXX-XX-XX-XX-XX-XX +#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p tunnel_type action +#types time string addr port addr port enum enum +XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 ::38:5f55:6265:726b 25977 2090:9090:9090:9090:9090:9000:: 25964 Tunnel::TEREDO Tunnel::DISCOVER +XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 ::38:5f55:6265:726b 25977 2090:9090:9090:9090:9090:9000:: 25964 Tunnel::TEREDO Tunnel::CLOSE +#close XXXX-XX-XX-XX-XX-XX diff --git a/testing/btest/Traces/tunnels/teredo-udp-in-udp.pcap b/testing/btest/Traces/tunnels/teredo-udp-in-udp.pcap new file mode 100644 index 0000000000000000000000000000000000000000..3f08887024b9bb8d32a068ded9ab794878de8948 GIT binary patch literal 175 zcmca|c+)~A1{MYcU}Rtfa`KC!6BaCFW@rbpK^P2}z$9ba-2@OLP|yKHfq_MQXi{oX tw!#EB01Ky9rsgmNa?EF7kOc9WQ&UQTOd|D3W|=S~mZYYHg0!YWNB~3@Bg_B* literal 0 HcmV?d00001 diff --git a/testing/btest/core/tunnels/teredo-udp-in-udp.zeek b/testing/btest/core/tunnels/teredo-udp-in-udp.zeek new file mode 100644 index 0000000000..25caf94c5d --- /dev/null +++ b/testing/btest/core/tunnels/teredo-udp-in-udp.zeek @@ -0,0 +1,4 @@ +# @TEST-EXEC: zeek -r $TRACES/tunnels/teredo-udp-in-udp.pcap %INPUT +# @TEST-EXEC: btest-diff tunnel.log + +@load base/frameworks/tunnels