From b21e6f72da7b674eb9f6de6108f22fb1d29d3800 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Mon, 13 Mar 2023 10:19:26 +0100 Subject: [PATCH] HTTP: Make Content-Range parsing more robust This was exposed by OSS-Fuzz after the HTTP/0.9 changes in zeek/zeek#2851: We do not check the result of parsing the from and last bytes of a Content-Range header and would reference uninitialized values on the stack if these were not valid. This doesn't seem as bad as it sounds outside of yielding non-sensible values: If the result was negative, we weird/bailed. If the result was positive, we already had to treat it with suspicion anyway and the SetPlainDelivery() logic accounts for that. --- src/analyzer/protocol/http/HTTP.cc | 10 ++++++++-- .../http.log | 11 +++++++++++ .../weird.log | 11 +++++++++++ .../Traces/http/http-bad-content-range-01.pcap | Bin 0 -> 2029 bytes .../base/protocols/http/bad-content-range.zeek | 3 +++ 5 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 testing/btest/Baseline/scripts.base.protocols.http.bad-content-range/http.log create mode 100644 testing/btest/Baseline/scripts.base.protocols.http.bad-content-range/weird.log create mode 100644 testing/btest/Traces/http/http-bad-content-range-01.pcap create mode 100644 testing/btest/scripts/base/protocols/http/bad-content-range.zeek diff --git a/src/analyzer/protocol/http/HTTP.cc b/src/analyzer/protocol/http/HTTP.cc index 318a3f9774..f9f32ba347 100644 --- a/src/analyzer/protocol/http/HTTP.cc +++ b/src/analyzer/protocol/http/HTTP.cc @@ -425,8 +425,14 @@ void HTTP_Entity::SubmitHeader(analyzer::mime::MIME_Header* h) first_byte_pos.c_str(), last_byte_pos.c_str(), instance_length_str.c_str()); int64_t f, l; - util::atoi_n(first_byte_pos.size(), first_byte_pos.c_str(), nullptr, 10, f); - util::atoi_n(last_byte_pos.size(), last_byte_pos.c_str(), nullptr, 10, l); + int fr = util::atoi_n(first_byte_pos.size(), first_byte_pos.c_str(), nullptr, 10, f); + int lr = util::atoi_n(last_byte_pos.size(), last_byte_pos.c_str(), nullptr, 10, l); + if ( fr != 1 || lr != 1 ) + { + http_message->Weird("HTTP_content_range_cannot_parse"); + return; + } + int64_t len = l - f + 1; if ( DEBUG_http ) diff --git a/testing/btest/Baseline/scripts.base.protocols.http.bad-content-range/http.log b/testing/btest/Baseline/scripts.base.protocols.http.bad-content-range/http.log new file mode 100644 index 0000000000..e5a448b593 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.http.bad-content-range/http.log @@ -0,0 +1,11 @@ +### 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 42226 127.0.0.1 8080 1 GET localhost:8080 / - 1.1 curl/7.74.0 - 0 16 206 Partial Content - - (empty) - - - - - - FMJdmJBUqlAAHLXAd - - +#close XXXX-XX-XX-XX-XX-XX diff --git a/testing/btest/Baseline/scripts.base.protocols.http.bad-content-range/weird.log b/testing/btest/Baseline/scripts.base.protocols.http.bad-content-range/weird.log new file mode 100644 index 0000000000..0c73c5334e --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.http.bad-content-range/weird.log @@ -0,0 +1,11 @@ +### 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 42226 127.0.0.1 8080 HTTP_content_range_cannot_parse - F zeek HTTP +#close XXXX-XX-XX-XX-XX-XX diff --git a/testing/btest/Traces/http/http-bad-content-range-01.pcap b/testing/btest/Traces/http/http-bad-content-range-01.pcap new file mode 100644 index 0000000000000000000000000000000000000000..9ca75e011ca33fbf1595ee1f286e9175a9617378 GIT binary patch literal 2029 zcmaKsT}V@57{}io#UrPVX#@pf9+5J&eHi8!YJN5;#3@rRB5=+*XLFOe2@2NGz`Mpm5#pic+01rk^SN8LYPq@wF85N82}%@-w3nV%r>_xEFo=LGWk z{9&TG#i|&UR;$%wEH#wMQmc2=U#+;kcAI+;FDp%zCRyqlb@=oRPKO76vj==`V}+rj z%m6<$*zJxH__EkoEK9P4e%qRfMF;v_yTkm{T~5O;Ex~MEDiApA*A2vyphS8hDmGs< zai=&yiFL;kA#p|*BU+NULzY#!YARPOHlN>Rb1O|=j~~*>63!cgc^YRnN6$>p=Ba09 zfinx^nSFiD&$A@mKr9MNq#x9=`JxHBMJG*A#fa)o!gME);OqrPj3nYwOH!59ciKEo zN43&7?stqT175Gb&*n4s*(PKOXSTsijTG}V#T7P_OR~ktx#L%4V4$7`}^XT>DG!Tc#31S9IY`&;$m3WuRb~2(WyTfEReyPMI@*^Hy zPd4I%i`b8V)!6C_Dth!akAJ^7ica7>d}SgQdIpFE?{!*d@1~;GBcgF8+JA--rCKD) z5;D!e)UbR>EejcwTjwrXIzRnq*2R~kS|qkV$kHH+!_=~i5!H3>Qp+5H^v^J&90Fn; p5``HpqWKkdyu^r{<6aPmw(C4m4k6K`NfbTDZgLMCJ23T8;$Qv*+2H^H literal 0 HcmV?d00001 diff --git a/testing/btest/scripts/base/protocols/http/bad-content-range.zeek b/testing/btest/scripts/base/protocols/http/bad-content-range.zeek new file mode 100644 index 0000000000..aed3ff959d --- /dev/null +++ b/testing/btest/scripts/base/protocols/http/bad-content-range.zeek @@ -0,0 +1,3 @@ +# @TEST-EXEC: zeek -b base/protocols/http -r $TRACES/http/http-bad-content-range-01.pcap +# @TEST-EXEC: btest-diff http.log +# @TEST-EXEC: btest-diff weird.log