From fbf9d53c44c5946e4de1ade754a3074d1d72a058 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Mon, 13 Mar 2023 10:47:34 +0100 Subject: [PATCH] HTTP: Reset reply_message for HTTP/0.9 OSS-Fuzz tickled an assert when sending a HTTP response before a HTTP/0.9 request. Avoid this by resetting reply_message upon seeing a HTTP/0.9 request. PCAP was generated artificially: Server sending a reply providing a Content-Length. Because HTTP/0.9 processing would remove the ContentLine support analyzer, more data was delivered to the HTTP_Message than expected, triggering an assert. This is a follow-up for zeek/zeek#2851. --- src/analyzer/protocol/http/HTTP.cc | 14 ++++++++++++++ .../http.log | 12 ++++++++++++ .../weird.log | 12 ++++++++++++ .../http/http-09-content-length-confusion.pcap | Bin 0 -> 1582 bytes .../http/http-09-content-length-confusion.zeek | 7 +++++++ 5 files changed, 45 insertions(+) create mode 100644 testing/btest/Baseline/scripts.base.protocols.http.http-09-content-length-confusion/http.log create mode 100644 testing/btest/Baseline/scripts.base.protocols.http.http-09-content-length-confusion/weird.log create mode 100644 testing/btest/Traces/http/http-09-content-length-confusion.pcap create mode 100644 testing/btest/scripts/base/protocols/http/http-09-content-length-confusion.zeek diff --git a/src/analyzer/protocol/http/HTTP.cc b/src/analyzer/protocol/http/HTTP.cc index 318a3f9774..190e2def0a 100644 --- a/src/analyzer/protocol/http/HTTP.cc +++ b/src/analyzer/protocol/http/HTTP.cc @@ -987,6 +987,20 @@ void HTTP_Analyzer::DeliverStream(int len, const u_char* data, bool is_orig) if ( request_method->ToStdString() != "GET" ) Weird("invalid_http_09_request_method", request_method->CheckString()); + // If we already have a reply_message that means we saw + // an HTTP response before a request and interpreted + // it as HTTP/1.1 already. Reset the state here because + // we're removing the ContentLine support analyzer and + // any assumptions about expected delivery size state + // become invalid. + if ( reply_message ) + { + Weird("http_09_reply_before_request"); + reply_message->Done(); + delete reply_message; + reply_message = nullptr; + } + reply_state = EXPECT_REPLY_HTTP09; RemoveSupportAnalyzer(content_line_resp); } diff --git a/testing/btest/Baseline/scripts.base.protocols.http.http-09-content-length-confusion/http.log b/testing/btest/Baseline/scripts.base.protocols.http.http-09-content-length-confusion/http.log new file mode 100644 index 0000000000..9bca082c7f --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.http.http-09-content-length-confusion/http.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 http +#open XXXX-XX-XX-XX-XX-XX +#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p trans_depth method host uri referrer version user_agent origin request_body_len response_body_len status_code status_msg info_code info_msg tags username password proxied orig_fuids orig_filenames orig_mime_types resp_fuids resp_filenames resp_mime_types +#types time string addr port addr port count string string string string string string string count count count string count string set[enum] string string set[string] vector[string] vector[string] vector[string] vector[string] vector[string] vector[string] +XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 127.0.0.1 46982 127.0.0.1 80 1 GET - / - 1.1 - - 0 0 200 OK - - (empty) - - - - - - - - - +XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 127.0.0.1 46982 127.0.0.1 80 2 - - - - 0.9 - - 0 19 0 - - (empty) - - - - - - F7s8014qOBt9Gyy3w7 - text/plain +#close XXXX-XX-XX-XX-XX-XX diff --git a/testing/btest/Baseline/scripts.base.protocols.http.http-09-content-length-confusion/weird.log b/testing/btest/Baseline/scripts.base.protocols.http.http-09-content-length-confusion/weird.log new file mode 100644 index 0000000000..b6c643871a --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.http.http-09-content-length-confusion/weird.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 weird +#open XXXX-XX-XX-XX-XX-XX +#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p name addl notice peer source +#types time string addr port addr port string string bool string string +XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 127.0.0.1 46982 127.0.0.1 80 http_09_reply_before_request - F zeek HTTP +XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 127.0.0.1 46982 127.0.0.1 80 HTTP_version_mismatch - F zeek HTTP +#close XXXX-XX-XX-XX-XX-XX diff --git a/testing/btest/Traces/http/http-09-content-length-confusion.pcap b/testing/btest/Traces/http/http-09-content-length-confusion.pcap new file mode 100644 index 0000000000000000000000000000000000000000..f846fe2daf35694c3d979e156488312b0c2dd231 GIT binary patch literal 1582 zcmaKs&1(~35Qk^ekj91e>*S@d2SKsgBoai;9+tErX=B}h(S!I=qNTB50_~=1{fd{0 zQm6+#cxn~@0C5imkJc~nB2-&1CA}06O@)FvNTqRRx^LVK=?)o4LU?}9%)W2F&AoZy z13&yPKS=mtykpnZhmUtaAHJt{RuB>ZnpM(44?OC*4^Yh>7movWl;$|4Y z=dUmNeL;CG`6Er=OKolKM4t8}0}xqzE69+Uoj4u|#*n@+v$y`GnM`y+evRZfCQ&3+8~MXXOwA@S>efU{^z?JSl~ z8gneYDu}jpe?vNEOQ#N0iP&8%n1coL$f?1?keNTJMirStRxrdv`zCAmiV%nPfJ>V~ zu%94NM`929Qag*aj~VY-dqWUy?SY2&A)(zlPeer~<~eW6TWpyUOowI8ZAr||`KL%M z1p?5EdeqKh%a}FJM7<@EPDDspYBeGz6InH+$ON83@b(KeKJfT|1=t=xhvP4~RSB%s zo*}Va4|pupy3MN1S9l4pLyiw`W1%`eBCV###Oy$(hvi4MY<@^&S#etuQ?E`VF}}Lf z>$HAi;?EXySJ<*~T8E=VH1pR+ReYJnqI$h_CEumU6ml6uw%4}q@G24_#6AZVTt&&P zO(Db4mq`45!SmYQZMobn;vUC;O=@IR5aSoUh{k7L#2?{kAAl0BcDd{(nrlcU$BZY@ Vy2p;)f@n{@j8l)frye1q`45qDT?+sJ literal 0 HcmV?d00001 diff --git a/testing/btest/scripts/base/protocols/http/http-09-content-length-confusion.zeek b/testing/btest/scripts/base/protocols/http/http-09-content-length-confusion.zeek new file mode 100644 index 0000000000..10e16c3da2 --- /dev/null +++ b/testing/btest/scripts/base/protocols/http/http-09-content-length-confusion.zeek @@ -0,0 +1,7 @@ +# @TEST-DOC: HTTP response with Content-Length followed by HTTP/0.9 request. This triggered an assert. +# @TEST-EXEC: zeek -b -Cr $TRACES/http/http-09-content-length-confusion.pcap %INPUT +# @TEST-EXEC: btest-diff http.log +# @TEST-EXEC: btest-diff weird.log + +@load base/frameworks/notice/weird +@load base/protocols/http