From fd6f9e470faee85d7e1158c6da8c578aa440837e Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Mon, 31 Aug 2015 12:58:25 -0700 Subject: [PATCH] Add a number of out_of_bound checks to Packet.cc Mostly this verifies that we actually have the full headers that we are trying to read in a packet. Addresses BIT-1463 --- src/iosource/Packet.cc | 40 +++++++++++++++++- testing/btest/Baseline/core.truncation/output | 26 ++++++++---- testing/btest/Traces/trunc/trunc-hdr.pcap | Bin 0 -> 6435 bytes testing/btest/core/truncation.test | 6 +++ 4 files changed, 62 insertions(+), 10 deletions(-) create mode 100644 testing/btest/Traces/trunc/trunc-hdr.pcap diff --git a/src/iosource/Packet.cc b/src/iosource/Packet.cc index d40941095a..9c2c70454c 100644 --- a/src/iosource/Packet.cc +++ b/src/iosource/Packet.cc @@ -47,6 +47,12 @@ void Packet::Init(int arg_link_type, struct timeval *arg_ts, uint32 arg_caplen, l2_valid = false; + if ( data && cap_len < hdr_size ) + { + Weird("truncated_header"); + return; + } + if ( data ) ProcessLayer2(); } @@ -94,12 +100,14 @@ void Packet::ProcessLayer2() bool have_mpls = false; const u_char* pdata = data; + unsigned int remaining = cap_len; switch ( link_type ) { case DLT_NULL: { int protocol = (pdata[3] << 24) + (pdata[2] << 16) + (pdata[1] << 8) + pdata[0]; pdata += GetLinkHeaderSize(link_type); + remaining -= GetLinkHeaderSize(link_type); // From the Wireshark Wiki: "AF_INET6, unfortunately, has // different values in {NetBSD,OpenBSD,BSD/OS}, @@ -127,6 +135,7 @@ void Packet::ProcessLayer2() // Get protocol being carried from the ethernet frame. int protocol = (pdata[12] << 8) + pdata[13]; pdata += GetLinkHeaderSize(link_type); + remaining -= GetLinkHeaderSize(link_type); eth_type = protocol; switch ( protocol ) @@ -140,9 +149,15 @@ void Packet::ProcessLayer2() // 802.1q / 802.1ad case 0x8100: case 0x9100: + if ( remaining < 4 ) + { + Weird("truncated_header"); + return; + } vlan = ((pdata[0] << 8) + pdata[1]) & 0xfff; protocol = ((pdata[2] << 8) + pdata[3]); pdata += 4; // Skip the vlan header + remaining -= 4; // Check for MPLS in VLAN. if ( protocol == 0x8847 ) @@ -154,9 +169,15 @@ void Packet::ProcessLayer2() // Check for double-tagged (802.1ad) if ( protocol == 0x8100 || protocol == 0x9100 ) { + if ( remaining < 4 ) + { + Weird("truncated_header"); + return; + } inner_vlan = ((pdata[0] << 8) + pdata[1]) & 0xfff; protocol = ((pdata[2] << 8) + pdata[3]); pdata += 4; // Skip the vlan header + remaining -= 4; } eth_type = protocol; @@ -166,6 +187,7 @@ void Packet::ProcessLayer2() case 0x8864: protocol = (pdata[6] << 8) + pdata[7]; pdata += 8; // Skip the PPPoE session and PPP header + remaining -= 8; if ( protocol == 0x0021 ) l3_proto = L3_IPV4; @@ -206,6 +228,7 @@ void Packet::ProcessLayer2() // Get PPP protocol. int protocol = (pdata[2] << 8) + pdata[3]; pdata += GetLinkHeaderSize(link_type); + remaining -= GetLinkHeaderSize(link_type); if ( protocol == 0x0281 ) { @@ -230,6 +253,12 @@ void Packet::ProcessLayer2() { // Assume we're pointing at IP. Just figure out which version. pdata += GetLinkHeaderSize(link_type); + if ( remaining < sizeof(struct ip) ) + { + Weird("truncated_header"); + return; + } + const struct ip* ip = (const struct ip *)pdata; if ( ip->ip_v == 4 ) @@ -254,8 +283,14 @@ void Packet::ProcessLayer2() while ( ! end_of_stack ) { + if ( remaining < 4 ) + { + Weird("truncated_header"); + return; + } end_of_stack = *(pdata + 2) & 0x01; pdata += 4; + remaining -= 4; if ( pdata >= pdata + cap_len ) { @@ -288,12 +323,13 @@ void Packet::ProcessLayer2() else if ( encap_hdr_size ) { // Blanket encapsulation. We assume that what remains is IP. - pdata += encap_hdr_size; - if ( pdata + sizeof(struct ip) >= data + cap_len ) + if ( pdata + encap_hdr_size + sizeof(struct ip) >= data + cap_len ) { Weird("no_ip_left_after_encap"); return; } + pdata += encap_hdr_size; + remaining -= encap_hdr_size; const struct ip* ip = (const struct ip *)pdata; diff --git a/testing/btest/Baseline/core.truncation/output b/testing/btest/Baseline/core.truncation/output index 9243c2f873..46d3eecceb 100644 --- a/testing/btest/Baseline/core.truncation/output +++ b/testing/btest/Baseline/core.truncation/output @@ -3,38 +3,48 @@ #empty_field (empty) #unset_field - #path weird -#open 2012-04-11-16-01-35 +#open 2015-08-31-19-57-29 #fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p name addl notice peer #types time string addr port addr port string string bool string 1334160095.895421 - - - - - truncated_IP - F bro -#close 2012-04-11-16-01-35 +#close 2015-08-31-19-57-29 #separator \x09 #set_separator , #empty_field (empty) #unset_field - #path weird -#open 2012-04-11-14-57-21 +#open 2015-08-31-19-57-30 #fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p name addl notice peer #types time string addr port addr port string string bool string 1334156241.519125 - - - - - truncated_IP - F bro -#close 2012-04-11-14-57-21 +#close 2015-08-31-19-57-30 #separator \x09 #set_separator , #empty_field (empty) #unset_field - #path weird -#open 2012-04-10-21-50-48 +#open 2015-08-31-19-57-31 #fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p name addl notice peer #types time string addr port addr port string string bool string 1334094648.590126 - - - - - truncated_IP - F bro -#close 2012-04-10-21-50-48 +#close 2015-08-31-19-57-31 #separator \x09 #set_separator , #empty_field (empty) #unset_field - #path weird -#open 2012-05-29-22-02-34 +#open 2015-08-31-19-57-32 #fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p name addl notice peer #types time string addr port addr port string string bool string 1338328954.078361 - - - - - internally_truncated_header - F bro -#close 2012-05-29-22-02-34 +#close 2015-08-31-19-57-32 +#separator \x09 +#set_separator , +#empty_field (empty) +#unset_field - +#path weird +#open 2015-08-31-19-57-33 +#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p name addl notice peer +#types time string addr port addr port string string bool string +0.000000 - - - - - truncated_header - F bro +#close 2015-08-31-19-57-33 diff --git a/testing/btest/Traces/trunc/trunc-hdr.pcap b/testing/btest/Traces/trunc/trunc-hdr.pcap new file mode 100644 index 0000000000000000000000000000000000000000..0ab12ee6c78e1c88093333c6638359514c28c750 GIT binary patch literal 6435 zcma)A2|Scr8$a)Cw!tu#QDo3XA@59fEo4_BBDA^;BO(mh3S&)*N=0Q!De5Z9(kIpQ zMX8&*A(fJHtJ|d_NvrES?~Ex+b-(lbo!{?$&wJkg^PJ~-&a<^&K79xW2tYUx_<+s5 z2dmr^KS+V4@EMyxGvlxANo90@``dn^zy<&dWp4{$K>~nRMi@{U&?H0Uy`HkP}%lL>0@cYW|9t?Gi zUm`?RccEAScBX)@XAngLiU6CFQ4o0@_3xo%3Q%YkAaET33U~y(fPX*5BlVZM8a`f= z91!!2)tX~_89CtX>tLL`HuuPX%;=>m3Q&%0o*Yk{r_PfGroaSbfOMWTj|dDw3Qv|N z%2Vb|2MU17ljPxnIFHH`j)NSz0trCC0|W=efdK@O0+K)pu8{!^ZWf266#3gl;E(bUga|>X z&>nCQKpIdWmnFr-?0f*NuQuf?UqXx;PV6Hz5Yw|jBb+O zna7s<@4$o(RXONF0q;e%lvaxb<0 zWcJ)Ur_^nllL2kSq=nTU9BMCXl|4XR$qnX{LRkXs*g_jzghz&I6?2s8R*ID^(FnYA zdL(%3>!lV49#^`Bkp_L6-pYgko(urfMiD517vf`r69_KBl;|CXg;o=@Ase zn`jOz#E<1i_wWf0^z-2ahKA7HSm9BuaJrQxIzv$s{%3%iqWs|vUyV=$29v?W-n~&S zBEe8Wr(?L%WI31cz$hOMi|!mA;1d!U%LijnQ5j;N4hbP3T;o~r+6npgk{DZ@2MJ(OXxm$E2T@+{s? zzmJ?9Ivu?CtQ~RBV)=Dx|AgMJ9_)7rQGLC2M(!<{UUOH5@)r-I->ZWT;EG(;cHXpg z4AeXoSKFX(WWSv4EH#+(?8=?j`)k^+47;okS~cRo@b;}O3dLL#86bUarJpBf$(s@U zn(}RORoVUioPuh?Z5IzwDe)4c3yGk;JBxR@aIzS~%$1ki%TuVw8)s zLsg;hR5S=QR3TArew#~aM{&jLw~p(?S#4_VA3k(Qqbk@2DS`q)jthd_zN^ zv2w_p4%Ch_dSWN8ekC^3O1i?OoF%&j`4VA)(N+ZZ9fq=&miv>*lLQp{ENTSeA zi9``R^7RQ9|MM}2&Eae`)7SS651u=Y+0YI85o{l(9xF;O0^TsoaJ>M=9JmKHC}tL; zY2*GH9Zdw^4nE;8*4MulVJy=L=J0YFlf>4g*&}Hq!8in+jcN-y#UTpH7%vf!8NNT@ zj6=0@aYK{RN5+naAcS}{7M&^l6-Cl$W5)>fjo3Kh7^v&lT_S0=!o&Jto+e_#Mt$Mc z!CZ~d(|mI@0zYGA35CGnUsM$;rU4uZCb>O47xVJ6y5qh4x8SclWy~5|=_`xt5JrsP ze_)b=02|er=l+&^zIoP_pwFDyp?A9Ko;(~`NSYPz#A5ai`%#v*7C$|y(NK#jSA!3k zE%+0OcXY0*A2e(T^6gOB)8khZDY9*eyIB7h^#!MBgr}%@(Kcr* zZcJ^TSya63o_kYI{vQv)FF~`D9ktgyIH^p2iep=6>_cZ5cFzCp)NhQ8H@z<=hq&Vx z0@wHN<7PI3#K;@koEhA5MIIk>metgS_cV8>d2q!+{87}SD%3+!9{MhywLL*9$&=K922XUP!YIVgkN8mt^OQnIIUyEp>nlrh zbD=DJhhDTtt1mT%g+*pWycIx*Z`b`;dQB!G3Xo*74mAf0(`^ zS5|`bHv!sdsaZRGI~%GjOnTbhy(}ft&dy)Zl&{EqNz3~5hJ4gdE%kR1ib17ohK?%x zKxqwrc^Zk!u-DbH4uf^TV99s|dkb82t z>FZDS-u3IqbAK^%YMM*%@&=i~5ywHwxdf@9I~R5+Y?XDt$MZ3GJ^WDbWn=m;Y1{9q zJ*{*iZ+&R0sn|7qH#hYjL2Y9}Xz1vwdXpzMzk96E-l8mNx#D=;S*>$bPe0A>SJQ}` zR-tzw%`ofWrkaR~d%1c~-;xVnUt0R>7DZlIq3)wMA1T^ObI061;s67o4JwOyEB^IH zMqyToIo#zVLWhf_{foneS;rHyL-pMGjFMpTtxjV?khiWd~mOEVKXtMFi901#wH zUpFG+lk=NWjPW;_HO>2zraA6fYHqtf^`GRDkA+!>Z%D2=GyfJLI$KUIj(PAZ zIJ&B_=8*e#Zx8kn^^NnA!tNK8*_aPK8!lPabv5qS7olWM-`P%*=;Mp&k})nja}LnV zcVdx19!7!$ykL%%fy^HaQ>+BViaC5(OCweXNVrX`1ClcN5rPKZjp_m5rS-;8n|eZX z%boYJFjGgm6@b(On7-VFM%*TjfMPCk13osB!~|IZ3gadh`43#GL6ZtgVAAqE?b4)~ zFp))#=Ax*Hp#hdSN1$P-11e@q)RDr&x&jd79EQ$k(k@1k@|qr5=&{Yi)<1ooHXh9a z2s5OqRYD?|LZMh@Y{?iMS3f|rLBQ=I%HGVti0L*sxeSZc{;u6i<(dQT62=v{UrW;s zQM#{78>^3su(M-Dy3bZCZTP%{x^Tg@FOM#rGZF2Z{Yqwpdv6x+=1%YFrexKqdF-1m z39VO4XsI&>0v;at@L<38mLX}^8+%g*$S+ObzCG3{6@9VnpPmy63i5GAg@ax3FSq2+ zJ+*2m_nxO!`bxXNBLRJH88Z5hQT}Vyub4w&Xzw9{JpGMC)x8mJtH0I>G1G|;$j%I1{IzPu|+CYcqObPeZTzNf? zQ@OcnMMQjEoPtxH!JS-Lr^`=A2Atpdp7pM85=*_A@HqEm$MJ&+Vejs~vcBjVLg!TM zGO;}`n*J($*6$jrcEOE@Hh%uS$05q?*=b(a+8{@9XPG?bqMKHr>`BwY0F$M5Lv}A; zi|?T2G|xXb=jFLr^Q-gM^i1sD5$uh1l8m9|Ii%pxzB4}A-|N=n*~87PtK zjEZ8eEb5NcTbaUi*q75EbeZK=e?;3F^eHBLuKAZb*v|G=&4tGSbn32*Itc}nrH{l~ zVv4Yi1wm@80$&Vorjqcg1;JBU;4fv;PhWxi&<~Yuop_DZOLK z{epyByC_Z{BfPZFyWKEbtt?tloo)VEtn4;^G^}}*$qVb>=(iON)ZX=XnV=EShx*s*}u7&dRQ{O*MtFj<;~0U!rUPHawYl_8{-Cd?N~o(Do_ zsK)P=rL;ZJ3=ejJ&hUJ--~h+rL5{-ASr-)lQNB`tQzYeH`ohphhHD7h?#El_*3_n) zNl=e*oWCdEX*bUALs4wSJi@0hNl2&Sv5SY(%?^zodoE(fx(a&b3(Zzt4BO{H&&fJ| zLm{zX!L8f>jf3OMih}$7J*>FV{`}b`t%bR7@5D+RwKwg5P+NNYL(CzA>&cru+Whxe zfJHl|v+ss)>v=n%ZS=S1$E~_6v&D2%Ru|{s;%~R_ZYtikx+h|vP*56RrdFZ)HHz2f zH(hVmQ&njm*Ou=LH9)eqTvOB;&i?WEvc|neDpGvF50haIl%de7iBl}8&;r^3yZ~6M zQDBc)Gc!E6uPn6++T)-w#idmcfjcxJY)-KxI|k=_mNbR}Es0$YfFl0hgn@u*F@L$Z z*R8t+_O-nWdsG1$U_-6}!cW#TIQi-drfDfTp`ry%(bOu)={8K!mx3^S1SIi>G*50` zY<`4iJ-3&5iDGPS6p+Ncxx3u|M90<(ylZ7a_xxT-ghUUxcSqw)Zb;ZW-r!tisBH=&rBjz|^x?wOr+&C2@2@gE!4scoq)4v+{U^ z0!}|#w^OF!z1JhD*4__)4}B>eKIi5bfDwp+1je-;u~uQ0Fhu~`EzNuac@u?|=E^4^ z1Ku^1VLHSHhSd$Rgyu0tvD&9+Pgbl2W*%4tWDL#Yz0f?Kb`hTc7)9E4G78_wFcepa z^0kiY`zbHa2F6fICYb8asiIce8*4}q)C%i%D=1psBu?I l1ygB_)g+a=VM=s?&P`$!_Jj=#OZjJ5+FCRCSd0E0>wgL>%KZQU literal 0 HcmV?d00001 diff --git a/testing/btest/core/truncation.test b/testing/btest/core/truncation.test index 3406879183..c0e4ee857a 100644 --- a/testing/btest/core/truncation.test +++ b/testing/btest/core/truncation.test @@ -19,4 +19,10 @@ # @TEST-EXEC: bro -r $TRACES/trunc/icmp-header-trunc.pcap # @TEST-EXEC: cat weird.log >> output + +# Truncated packets where the captured length is less than the length required +# for the packet header should also raise a Weird +# @TEST-EXEC: bro -r $TRACES/trunc/trunc-hdr.pcap +# @TEST-EXEC: cat weird.log >> output + # @TEST-EXEC: btest-diff output