From 917d03e4f9ac8cd343c7fa7f4a43165b93e2d8fa Mon Sep 17 00:00:00 2001 From: Charles Milette Date: Sat, 9 Jan 2021 20:25:26 -0500 Subject: [PATCH] Enhance detection and diagnostics of LoadLibrary(Ex) --- flawfinder | 22 ++++++++++++++++++---- test/correct-results.txt | Bin 8681 -> 18252 bytes test/test.c | 6 ++++-- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/flawfinder b/flawfinder index d5c0aa2..89f29e8 100755 --- a/flawfinder +++ b/flawfinder @@ -847,9 +847,23 @@ def cpp_unsafe_stl(hit): add_warning(hit) def load_library_ex(hit): - # If parameter 3 is 'LOAD_LIBRARY_SEARCH_SYSTEM32', it's safe. + # If parameter 3 has one of the flags below, it's safe. + safe_search = [ + # Load only from the folder where the .exe file is located + 'LOAD_LIBRARY_SEARCH_APPLICATION_DIR', + # Combination of application, System32 and user directories + 'LOAD_LIBRARY_SEARCH_DEFAULT_DIRS', + # Load only from System32 + 'LOAD_LIBRARY_SEARCH_SYSTEM32', + # Load only from directories specified with AddDllDirectory + # or SetDllDirectory + 'LOAD_LIBRARY_SEARCH_USER_DIRS', + # Loading from the current directory will only proceed if + # the current directory is part of the safe load list + 'LOAD_LIBRARY_SAFE_CURRENT_DIRS' + ] if (len(hit.parameters) >= 4 and - hit.parameters[3] == 'LOAD_LIBRARY_SEARCH_SYSTEM32'): + any(flag in hit.parameters[3] for flag in safe_search)): return normal(hit) @@ -1298,12 +1312,12 @@ c_ruleset = { "LoadLibrary": (normal, 3, "Ensure that the full path to the library is specified, or current directory may be used (CWE-829, CWE-20)", - "Use registry entry or GetWindowsDirectory to find library path, if you aren't already", + "Use LoadLibraryEx with one of the search flags, or call SetSearchPathMode to use a safe search path, or pass a full path to the library", "misc", "", {'input': 1}), "LoadLibraryEx": (load_library_ex, 3, "Ensure that the full path to the library is specified, or current directory may be used (CWE-829, CWE-20)", - "Use registry entry or GetWindowsDirectory to find library path, if you aren't already", + "Use a flag like LOAD_LIBRARY_SEARCH_SYSTEM32 or LOAD_LIBRARY_SEARCH_APPLICATION_DIR to search only desired folders", "misc", "", {'input': 1}), "SetSecurityDescriptorDacl": diff --git a/test/correct-results.txt b/test/correct-results.txt index 6f726f315889ec8139fe5342b5434380f3e3c131..b7f69ea1c63ab234cf57f7bd6cfdb83f13441343 100644 GIT binary patch literal 18252 zcmeI4?N40C6~^atrT!0_4^ePZuzAO>s@5e$Mg+c+xHspgPz>c_lJ6>lU}5E8gExKG5SpU zWBNmSoKDkGdYfB1nq^P@PSi%9zP?YT_qsk%pEvpQFrBM)M_0YHpPxEbZ+h0PgFMzily@`jSM`BY z?!{<7q;;+IVcO6rPc`;-dam!!^!Xrr&{G+UNtLX z{R6GtW6<||anOA0>Dxdp-OTfD`bppS)P_{+ap1Y7yeFO?O9RaHRY_}2@|?{QMb^=R zgzsc|K3Ct}JZ?}jm9+tW_49h(NK%$_b9GA^?-D)lAmdalLr*NV;~a=Zd`1S9l2DV$@F7#h=ydNWG6P>4K(T za{qz)cSPlmevs3vv?)nXi|=S{PknYSBpN(5m(A5xJq`b`99y9|VLrUp9i-v5!wc@X zrFUT~J*{C+y@F$w`kTBqRtY~3@{@-bJYsF8Ki2N?ebvv>eyV5sS&#VaNUsjW)pzO7 z`aZ~7*3oFYqJ2MKcSH>~Ya2xVCzlkx(Y^O-;XY%TW1s+hGsmq%aBOfXIjG_(FvVDL zmE3w+K22=F9!wP)0!{r(s-S*Pt?e2+d#%R4*0WuWH__S9&+Bv+%?vI567&=8t2Hxd z`@H&TyBe>V3)gDqrYzx5+If)vF8_(%Ka>5u&@~vddSMd*q(K2{_T@Nz4&i;&Iy$rX5k1CUVP~SMxo1O1y@YzC5`tr{xj3 z9c5~}+1qbRPX^L_VvtkW4;p)}$P7E`=Ii%r0r`lKyRyq-caPP}(HK4ujYF&PijQ-| zNc@O(ZD=g~F5Z#91N|`8PwGpA2kvr**a+_0YK&rQ7c-paID@gLGIc#s$>_ucq0tXD zQdeX2^>ZiN@|h$??C?%~&olzKf;TqZ%;_let$NXpuZBBB(T*vgHX@QTx^oG~oXa`- z1reZw_46e0^aJsJ!u`+3{n?893uCepOSvmQdR}SvZDl6tpHZC1k2xY@wRYprpnz61 zEi%{k7(DcxJhskv#zo>}8!;boo^#WPw@cImT@k5U*Uc5;b?%|mr3r@9hw2lxzh!0Jkw;{xw za@ODT>Py`qWN$=v-bX&#I?6g-tY}^p zB3sd>M#3*$Jr>qnXMF9Foga~GO&@D(*Xnhx(3uTQ|L{2k5@PT+UV#aY^ zmmJ)AFnyY%?J9PDuhEXhS2(H@e$NU+2GIHL*#-WcYQK`SDJCarDi4 z57ov9#4TuQ`&xL~Bf4+N;%n+ZrMk6U&26bSX|um!bwNlap~hQ{^xKa-LCIm<7~QME z7lF3c9q`wB(~O$kBkH#xRPjx&u*SU_aoS|Gm`Xik97-LE2IP;=sB8MS6?HbVgQ&x@ zD*K(-$Duyoo+*U(x91B@3y7T%o*B+ki@wfZs4_fvimK4KNe5OT@V@IkJA%X6uHOx zy{|C1q^zl@)I1zb?o^(c2$jf=JsHPx#1_;{;5zYIsbvuPdN0hJIGea_g7U7%b+suk^C-_K)5W*V5?Q9Tu;z;5xToPJ=BA9|gwZU}Q8sxtao z99q}D3HusP(@R}%i%WcGzk{on`enakGuJHx1)LFl>?jY@mVTINRbxNX=M$mcYt6r| znF24M7<_aEkc` zpfuS}%!Y*7M|VxYx@+I-{2srHhl)KR<04)P`67*d{SqJA8}N<0RLWeDsHtqr?}GnD z3$8lDu$sNH&qrKEIWtk+4$AB!%F2rVqCt_rh84uFM$l%De#{0 zfJDP}uvm{XrgnWOk-~U=sJT=u__pHiVOw)_7zhj67a4_2hHJTDL180p)!t9#GZ|4t z(4(BjpN!k%ZsYudPG&_`I*#)oHHck!zAbw;ZlD)<&vTs(3WNZ?=}(+UPCQ&mG)9px zxcX)M$3I%TiI>0&dsI*uA8+jmFY;bb)2GyVgXe8$!E8G`!Ojc2i)brRfg|Ty_MH6t zDxa_$-A&obPe)Y)uZ;zeZ+exC4i1}Ew80)dXdBLWc?y~wk<_Tms^b(=Xcalp)v-)S zpjfao5wF=Z;Q`EtplVoW;4R!C>Vb~tNr`qXDtYrni#TyO(W0Ac(PFjdH%^O))sAFi z>@F5Ox-B2jti3Qt6Ls3_Ep@bBPCSlka2`Fu07u_fVZf|# zb(Fhx9fgR7SgcoJYMoUUT#SB!i|o%5BM`|<>$f17acsEeNR(GsLx@z!nWuE3i~V}+ z@LUK_EYuTj`pi~fX}wO_xs|;I@ZOw(ulNL>BRWRw>)l$% z=0u9n#(0IQ7|&61KmAobN~All9DFXM2n){3$}JYFkmUfn)A7yT`fR;h!2Ia&Hu)sT{{v${K;EVav~HJG?N^}1;|EoB z4ji-ijWLk_4`G?Up|#;=Quvu-eUK^;ONb?X*k9~+M_z> z$Yb*6;4j%4*g>n&u6+lC*eUYrt7Yg)UXkEvtpW_BhUi*bb8h3YF_I_~JCA**aY(R} zQ3UL3zI(#%Peh4+1GMZzx)K|h@0hdfOikyR{m$Z)xCAPpNAczaUZ_T1VqZ|trzD^Q zUqoc!s%_MF*`aHCiKQ_S>C#7F%m3>eVt!ku$in+B__X2`@q6)3YV-X*dXaCGGr!2j z75fs;f$pI<<*l){iqUdag&U{Y6Hb$B5iQ+}(|F#87jMzCZQKn>9+P{=9S?=E|G^;piE zS9PCE7`xooTu-!@kCig+%EhQb*WWT?p}mYuk5|&2AN2_hOZnOwM9ckr?Rr_c_P#yW z3;DiJ-SPareC_`m!2RXCqPDsDKL|1VcNg<7s^S4Xv-&RyrU?GgGFP>Dj`_N8*)mMY-Sm%3PSqS4rD`e5a7JUz4^hW1+`Q_y3*zxggGx@QuISE+?wgXJ zc20K|bZ1ug%V=g z&B@qP_*Sd-*`{xEzsUWdfcF?Ut%@~!H-04x2iIHb zdn~!35lAeg_(g_ZsniH5_X literal 8681 zcmeHNZExE+68`RAG5eutvUMC=w)0jLcR4qU7MG+&oT4ati-MMDo2W!;MJlQ9*Y7i= z-kfZ)!EW!`-tDI-5;+_WXP$Xx=$}$?yHq-n1>MNPDx)bH3b`Stvy*9#+0#?cAU z`{~cLvnZEKS?tnEI(yP1dTyYUHjd)8j6c%S6cmP#!B&^4xrNTNw`Y5!XxJxyh=#iZ zdSfLmS;h`)Yn>FypkC}?cy}@D3LOi_>zxV9doQg_FfleoA!COZJ94lZ^tWEcCd*6b z_q&!<4W&$cQdyZnlvV$hA8enWQas{8 z<1DmW_8|+~7Ns+pa4Hrl^eH67^V%4WKXyP@%6=qeX{}c4r{thgD)q{(DHjEf&B+3C zM4n4gSXOHxNkD5DM`)~8S&2PidtDB4%`s`}n z_kpv(gRh;Y=%aBQ*(2UP>Ra7rr-$P}GX>3<$%})=gUf zIQZY(*WqEkuT0r^m*&#VWn31@ZJtXT7bpt>!pLFsC<7d8kXvVl*!(6e+Hy*tWB}iCG16+~g zI!Fy`{}xu4)&p-1&mOz;4P!U#?cqB|!nhR$C&LqdqoEsw$&g&&E1X~2qlkP)+S*Ql z{)g_Tqa|lP8)o}#(rD?S@1{;FjT=NBy1&xeKJD|`a9!lK-CA?r_iN*%c1on|*Z5dv zI?4;PNM%-?%yKVbXBgI*95GhmZ$&wz=I{ai zWa=7}Zgw;E{Gkg{DFc3>><=jsD!^w(DjDawNZEpYEZqVA4(!)e!=X3m7+7W3GEH}B z<`enr!f-kc@d?5d3j~Kn%xHPbP{kj4DftP!0C`_%)D_l8j*7wU_#20l6-$|Nj_^=^ zcm!0d52DBKm#VM`_hMCM2*W;IzPe@#2FmYRV_FF`Oni2dsP>Ws zaAtD>)o=#P9#B!3+b+DZR&c8LZkS_3D-;v_xhPxE`pY(ez|Yv`*h3)uO&y3ROHJ-R zqz2>whcq{tB$ILxsInZH4F~Dr9A08PgNI~?DwM=ZaFvF$M-8Tt81JHZ?LOPJ@XS}v z+p-Bnhh^d9tJDDq2|0t^sv`3UMhvJEug7@X#N^1wWYpQ%cgew;a)?7}YM^L1D`VEvtA4F8u-G_InDi5&s>HtA0dPAU2v$JkRKA&SjhTNA^D7 zWY}Xj06G+f*tC==gFAvqgdPOUfrbFEB}Ip&Le;z`Zj3-plLr1};978UYs;n}9UcF$ z-*!V@qoV^BM+;Nx1S_)RLTH~662j`D6M)KfYH-)k%Tj zQf-vnSgXJ$?^)-bZ~tYTb>llzU#@h0CzIJ1q84pH;FvtG_h;HI(3qQx^p<^c4czPo zYa*Oz)%1-2+gH=~e7=1(?*YaP{Ov+!ruZUd_ZK|)(XcuB(3V*Oury;^NQIsZ@+$_c zrQ){to@eRoSam0+kO>-W$hs(_c5aja!Ol{pKce%(P~^!Gr^;Fma;2}fxL$=wa+@4| zo6t2*bb)rI9heubK6siE)QH_E> z7oidZETKuyA0UZ`fjrp11rj+v=yZEV{`f}!U)j3pL^hZ(@+I7s8NPbL-ews0b&h;8 z+S0i$qF*|co4A}o_C;b1rPCu@WyZQh*y@X5F`F@|liHN4HN2(`yed4J{23A~&%mXO ziWPhdS>I3h`IhSz(KZ1f>oaYDspr63I|w~7j>*s5xI6{>l~z9n?G0G}ui+kOFIt=2 zJ_Va-FNBo75vk(pF*lwY+uy0>ruV!cTvvn!;@42f^T>M!`p&PU>eBhZxB2eu4!n&| zzahy0vIMXh2zJHr1lSzk{dJE%dCHC+?j^W24tUN47tY|Mo;(7?nRV`=JWpr`pF@os zh&_mLAuP)0CBmkvtI&z9gW#a;$sXMEk^kUT4EOo51(YDZu~^^cY~R))o*QroccT7M zropyDPs_Sy{$@4qo#Gn!-TdX8=5H=8POskKD)YXc%lq^u;Zn*wo5X(Q$8H<~eTu_=0*J zOcv1&L-F38<{Kup(uS)bLw1dl791tp?OotT2)EKy@Fj6=n^CWKiRj`<836s)mwZKGB7MYvE`7U|l3qdYHGwx-CRux2Z zbe{C~_i5SuT;YU*wmr}Ee`KovBO)3}4Xs0wVpFKlK!R@M56zmq)j+)bb?^Hh$}$T(6!sZ!JvGCHRn>&V}GM3>2>IRApx!E#jCg1uO9by z*3RYjWPd*arS!k2z@GR00&)-N!o4#5Tml;h=Yt+-0sCa^8~(j(f9V7Uiq9GyLeM>k QM&A%LQ=Z(9f^pRQ6@r#xT>t<8 diff --git a/test/test.c b/test/test.c index 5405624..10adc64 100644 --- a/test/test.c +++ b/test/test.c @@ -77,8 +77,10 @@ demo2() { SetSecurityDescriptorDacl(&sd,TRUE,NULL,FALSE); /* This one is a bad idea - first param shouldn't be NULL */ CreateProcess(NULL, "C:\\Program Files\\GoodGuy\\GoodGuy.exe -x", ""); - /* This should be ignored */ - (void) LoadLibraryEx(L"user32.dll", nullptr, LOAD_LIBRARY_SEARCH_SYSTEM32); + /* Bad, may load from current directory */ + (void) LoadLibraryEx(L"user32.dll", nullptr, LOAD_LIBRARY_AS_DATAFILE); + /* This should be ignored, since it's loading only from System32 */ + (void) LoadLibraryEx(L"user32.dll", nullptr, LOAD_LIBRARY_SEARCH_SYSTEM32 | LOAD_LIBRARY_REQUIRE_SIGNED_TARGET); /* Test interaction of quote characters */ printf("%c\n", 'x'); printf("%c\n", '"');