From 8be8c22b3e7ede0f6bb34bee85f081cf841b9027 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Fri, 27 Jan 2023 19:22:09 +0100 Subject: [PATCH] smb1: Prevent accessing uninitialized referenced_tree The added pcap was created from an OSS Fuzz test case and is borderline valid SMB traffic, but it triggered a scripting error. Closes #2726 --- scripts/base/frameworks/notice/weird.zeek | 1 + scripts/base/protocols/smb/smb1-main.zeek | 9 +++++++++ .../weird.log | 11 +++++++++++ .../btest/Traces/smb/smb1-OSS-fuzz-54883.pcap | Bin 0 -> 14907 bytes .../base/protocols/smb/smb1-OSS-fuzz-54883.test | 10 ++++++++++ 5 files changed, 31 insertions(+) create mode 100644 testing/btest/Baseline/scripts.base.protocols.smb.smb1-OSS-fuzz-54883/weird.log create mode 100644 testing/btest/Traces/smb/smb1-OSS-fuzz-54883.pcap create mode 100644 testing/btest/scripts/base/protocols/smb/smb1-OSS-fuzz-54883.test diff --git a/scripts/base/frameworks/notice/weird.zeek b/scripts/base/frameworks/notice/weird.zeek index aa7a412bbd..ee8b850dd2 100644 --- a/scripts/base/frameworks/notice/weird.zeek +++ b/scripts/base/frameworks/notice/weird.zeek @@ -206,6 +206,7 @@ export { ["SMB_parsing_error"] = ACTION_LOG, ["no_smb_session_using_parsesambamsg"] = ACTION_LOG, ["smb_andx_command_failed_to_parse"] = ACTION_LOG, + ["smb_tree_connect_andx_response_without_tree"] = ACTION_LOG_PER_CONN, ["transaction_subcmd_missing"] = ACTION_LOG, ["successful_RPC_reply_to_invalid_request"] = ACTION_NOTICE_PER_ORIG, ["SYN_after_close"] = ACTION_LOG, diff --git a/scripts/base/protocols/smb/smb1-main.zeek b/scripts/base/protocols/smb/smb1-main.zeek index e489a5e4e3..cb6831d708 100644 --- a/scripts/base/protocols/smb/smb1-main.zeek +++ b/scripts/base/protocols/smb/smb1-main.zeek @@ -107,6 +107,15 @@ event smb1_tree_connect_andx_request(c: connection, hdr: SMB1::Header, path: str event smb1_tree_connect_andx_response(c: connection, hdr: SMB1::Header, service: string, native_file_system: string) &priority=5 { + # If the current_cmd does not have a referenced tree, then likely we + # missed the SMB_COM_TREE_CONNECT_ANDX. Report a weird and stop. + if ( ! c$smb_state$current_cmd?$referenced_tree ) + { + local addl = fmt("current_cmd=%s", c$smb_state$current_cmd$command); + Reporter::conn_weird("smb_tree_connect_andx_response_without_tree", c, addl); + return; + } + c$smb_state$current_cmd$referenced_tree$service = service; if ( service == "IPC" ) c$smb_state$current_cmd$referenced_tree$share_type = "PIPE"; diff --git a/testing/btest/Baseline/scripts.base.protocols.smb.smb1-OSS-fuzz-54883/weird.log b/testing/btest/Baseline/scripts.base.protocols.smb.smb1-OSS-fuzz-54883/weird.log new file mode 100644 index 0000000000..31de8a0c7f --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.smb.smb1-OSS-fuzz-54883/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 10.0.0.1 38511 10.0.0.2 139 smb_tree_connect_andx_response_without_tree current_cmd=WRITE_ANDX F zeek - +#close XXXX-XX-XX-XX-XX-XX diff --git a/testing/btest/Traces/smb/smb1-OSS-fuzz-54883.pcap b/testing/btest/Traces/smb/smb1-OSS-fuzz-54883.pcap new file mode 100644 index 0000000000000000000000000000000000000000..030189ce6558185d92a67220ce5265e5d82568ad GIT binary patch literal 14907 zcmeGj3s{s@_Rb750y4Z5(a3zJsiA-(ZncW|NKtVNY_YV#fx*a`0hvMZZER_qx~cdm zADDe9O1>YNxgSWr)>3+Cgj!i`^`T}i_8_&KJ?H%M5BL8aftg^nT@Rf5|M&d&o_p@O z=bU^0d;d9gaOXND^I|pN%@mNahP>no=wx`gwEU$wUR303H5*u((b^L=7a0-J3(EM# z^b*wL;$uI8D^vN5lhOlXi>%I4E}5rkP!)Z6|X`H4E>X+5>$<2=uvei zQ7x)ISip|O+=bsO`1OVU>}6;ak$@NcMWle>CYj(iAd}$NG*l z>Zb5NMMN<6(8#1LGZLEt21v32ZGa>y*k_+VTLgJT0^aWOkOFerP`N0p!yirpxf~VT zJ{L=9W9wxAy*)92eqZ>rZ=elC0$z`wAO+-WBhzC!h#HRa8M;y(S9&zi6W0bhXuv>x z8VIP>Ta8w0My5qCHBh0pkcy7S+A>wk@im}esU{qoY-3htHo77|C@>1LY|WrN(KLXi z!$2SsaGh2pBL#v%fwa?#=fzGJ9Hcs8r|WXS+WY5F6LI`qNe3TU^;lWHk+bHh)=4H9 z%$vW>sP*@I&*G-%jO(Tr;Hj> zGfp5LfhdMYl^e7YNJ3HE&PQ>llVB9D-H1>a(Mcq;Q|*xpTx29TO-&$kMlKsn4RH|0 z3Sfv33Ao`>QaFp%q@_!T%j-UIxMr=PI=qua1Lan;l&k*XoGSr-z)&gX9Fr|0%Vg9~ z7@s96*qE8924EiA06gxpaB2Xpbxb^CBY=WPz*#HnhZLB{_DZc)Ggq|MzOYvOEY_}L zY`jpLX{{!G+SB?}6EdHi19JsD5TXv;2$m=;9v$;VI?fl|B(4Z&OtS_j$!|juTrA8;vEX)wtIX ziuxd-zQA`s`w(Lff<6%mI6;))@hC`Ig-noFMM0JdTI-K%t?ox2A_hWY&P+4pSoKq~ zGgx?ZpO_d1`bSWx=rV`W22V&Cl+e1NF8+P%D)4{(++u^-zGu{w%)zE)yF(Ka*I9U( zvDd*~5D7TjQ7lIaO!_HO+fe~saGf4!sm`5hJ2sqotnjZ-7p!V*qd|?;a(C6-y>Kfo z0&jY76JhM%NVW0=!`;$*6mEw*HEb;iW7cdi@^nAktsbOsJKU+^&DRlRXE|GqqrQ3` z?$&21*kCYIL;~JqDTso}7E4W5Jw+5`F}>Qb+i9|q4!P-4Gw_WX?&=H#`&TL44g{Kc zqE;WgK%KdrTIH3>WLo_n4FqLJ6l^S5KOzC|2?=e4#Cd3)x z^1Vt{2`wNJ@WP!z3J7FK;ik|M88!;N+0FZH27w`1mavhzf`3PJz60NejUw!K z&=4gH1JMu(xPB|NNC5{pFYR}e4Pw6s9-%t?J8*3~wXWWQi2)^YJ-Jkqi>(5F!Ck}6 zlkEaBSeJWhs4BHp$&x`$hy=W<;*kQX+ALL-Hyk7g_FLS9>eQ_&D&|IVi|)I@fr{F1 zxLc#P^0jZvK+RihD5IW7uT)AmlsP@KJZu$Q5jT{%IN`HJ)bo9!D>llB0?Ie;u+r^~ zd0!ZP!44|Bk={f*;7>dnaAO|jobhiZYY)0ZB;fR@Y>pIosr^c-N8b!lPAxb&Kw|v@ zu4HFkwgW(XJWFJYkh+*O(wl4pd01W2H~RBBSFkg~RBRoHfk+@kI$;+mA1UB@sbDeO zE|5Hx0ng-Ygsq1hLO?=v5mD^`Kez0xiGZzI;c#Jyf$`uXtjdW+1Ad3zWhCvWm5Vb`c4s~cGJqXZ-kh9eo^He2X{AU4!N$8z@j!*Z_~HW-KIEjHhuvN~hTl9cCOOE{|h z;mLz#7sh>m>U33j!Z$v1FE&4VJTprhz5CeE@>Q0yF6vGN#TABGPfp!CB;vtPzs{e} zn0aA!lLv+sRpkZT*RC|8#FXD{*1IDt@9fI&d2wLgx-mtg5~|XE^{w1;&l%myLFWuR zzS~^TZu{fL0ePiCPvq}xSL!|J>j~P0nZB#u$XwrSPuoMfH7Omh?5bS2y&$Cggf;tx zu%)Ax=nLC__;|;blbiPbsnf`kZaF_+#} zuATfizS)8|rzRYo^zGhVWzSSbfBI@j>(wC+Z4F&k*du)`v_5}}VTf?^5rh ztNWIeblJA$c&C<@16|4@OM?FK*k|*#=l1+%Wy+7+CeNL48qH;G3omvFMi-HQGZ)1< zq`<3bsMK7V!e<47xvc(;>MX+ZtDM*KMaTimG=k-(LF(YEotTg`e8iYUoiU;}Gg+;D z^u}CUb?tB?vAW~dgBQ2t?%v>+-}K|nGlxcOZgF;dhn#O`F8%Yzxu2^-f)~8k*EFN_ z?@2qtA8ysRO>3XIy%+vAqmOAYSkI)Yw?F#GdykBY?iTDff8UPbUxiOkoKU=CXW>2XU!NcE zb7UUrebSlM&koN&KlAjojxDDhiaM@M{_BN4Wm{WKySP8M)0@kUT}v0e6cg}@erCUA zrCuG*#?Odc6FhHDmyRu?w|zLN^UA35SB-I_3Nx2oHGj3Led+e(jpklCN5*a1du|e* zdIx;B`HL{0)}PEv{(5c2^M*MC2f!x?2`mZHM8N)_PBY737CT7|9~J6&{w+N(WDay+(6i#_bf;vSRp9=r2_$%|cq zu!l$>Tez;%Z{Pb=r)HrASz7aSWzO#sadQG7fJ!z(LfdA< zSD#~5X1D7pcfC%1j zB_ZJ+uky(r%Q%87-ZJ!nPgi)c&)^=Bfb#%G_6-i*S?U4JKN3A){35E8%uoIrt_`u* zPznV?MdiX36@-BI1fYCTu=*-7%DKwH6;J(EIxm%YF*P)VNFZE=J%?(fz|>_2p}9Q= z?KZ|jrlPwSksk>w9ivdcW~BokJ%gV#)PZ%)l9#({r1R-*7rfj)pZ1?6D&^W`)P(!z z(_2H-Zl6#4Zxvg$cXqZKM;h|{e0qPZn!N>ffJngEgrW#3U=zcoHsN0-3X(g6>fA9W zsU%HfCm0u1DAQ^^-g*Csp~r89+U?L2^rbjv z)eCUU+z&me#cK8r)I=oUJf7`F3QT!%QjZS~5?j^L(wAEGB=iU^dXT~tdCPAbXP;NI zZ0H3d0oNl%8d4yJEs^yIPP>J9?aE>-@!#(ew))2d=?^RLYkXWr?P;!Y`)_xi`6s!Z0fLrb<=+y`;Wj|O7citIt zR+MwZOseBNrgI3){ea6e*%VrxWmwWnz+BE+_A4!F9z?(SS6a9G+lH;rXxJqX50QXV zEIWr32r9TM!YQ`J_o8Ck&7(S^0l4TNJ;#>`7at`!o=GpaE?Foo2qJL=w|@zc16E~! zMdS8Q9z)kt=UbImD4nkX$jf`BjF%vtutt9XB?U#|&nLMxdb888cenw6rl_1F_62$Q jdd@2dc+KW{`N?_l1I)|C4RGiN=;Imma3y@vD8PRJN@7VU literal 0 HcmV?d00001 diff --git a/testing/btest/scripts/base/protocols/smb/smb1-OSS-fuzz-54883.test b/testing/btest/scripts/base/protocols/smb/smb1-OSS-fuzz-54883.test new file mode 100644 index 0000000000..6c1206ea2c --- /dev/null +++ b/testing/btest/scripts/base/protocols/smb/smb1-OSS-fuzz-54883.test @@ -0,0 +1,10 @@ +#@TEST-EXEC: zeek -b -C -r $TRACES/smb/smb1-OSS-fuzz-54883.pcap %INPUT +#@TEST-EXEC: btest-diff weird.log +#@TEST-EXEC: ! test -f reporter.log + +@load base/protocols/smb +@load base/frameworks/notice/weird + +# The traffic generated by OSS Fuzz is broken to the extreme, ensure +# the analyzer isn't disabled so the original scripting issue triggers. +redef DPD::ignore_violations += { Analyzer::ANALYZER_SMB };