From 917d03e4f9ac8cd343c7fa7f4a43165b93e2d8fa Mon Sep 17 00:00:00 2001 From: Charles Milette Date: Sat, 9 Jan 2021 20:25:26 -0500 Subject: [PATCH 1/3] 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", '"'); From bd3787e2bc1508835502c77c026aa199095b65d3 Mon Sep 17 00:00:00 2001 From: Charles Milette Date: Sat, 9 Jan 2021 20:37:20 -0500 Subject: [PATCH 2/3] Update test files --- test/correct-results-008.txt | 4 ++-- test/correct-results.csv | 5 +++-- test/correct-results.html | 26 ++++++++++++++++++-------- test/correct-results.txt | Bin 18252 -> 8952 bytes 4 files changed, 23 insertions(+), 12 deletions(-) diff --git a/test/correct-results-008.txt b/test/correct-results-008.txt index dfcc58d..be8316e 100644 --- a/test/correct-results-008.txt +++ b/test/correct-results-008.txt @@ -5,8 +5,8 @@ Number of rules (primarily dangerous function names) in C/C++ ruleset: 222 ANALYSIS SUMMARY: No hits found. -Lines analyzed = 123 -Physical Source Lines of Code (SLOC) = 85 +Lines analyzed = 125 +Physical Source Lines of Code (SLOC) = 86 Hits@level = [0] 0 [1] 0 [2] 0 [3] 0 [4] 0 [5] 0 Hits@level+ = [0+] 0 [1+] 0 [2+] 0 [3+] 0 [4+] 0 [5+] 0 Hits/KSLOC@level+ = [0+] 0 [1+] 0 [2+] 0 [3+] 0 [4+] 0 [5+] 0 diff --git a/test/correct-results.csv b/test/correct-results.csv index 2b556e2..7770622 100644 --- a/test/correct-results.csv +++ b/test/correct-results.csv @@ -18,7 +18,8 @@ test.c,49,3,4,buffer,_mbscpy,Does not check for buffer overflows when copying to test.c,56,3,4,buffer,lstrcat,Does not check for buffer overflows when concatenating to destination [MS-banned] (CWE-120),,,CWE-120," lstrcat(d,s);",364b4c512862fdccbca27d2fa7737995b5d24b637a760976c940ae636218d340 test.c,79,3,3,shell,CreateProcess,This causes a new process to execute and is difficult to use safely (CWE-78),"Specify the application path in the first argument, NOT as part of the second, or embedded spaces could allow an attacker to force a different program to run",,CWE-78," CreateProcess(NULL, ""C:\\Program Files\\GoodGuy\\GoodGuy.exe -x"", """");",3c712b38d0857bde3832d85ad35ac9859be55c5f5f1c20af659a577dd4d0acbf test.c,79,3,3,shell,CreateProcess,This causes a new process to execute and is difficult to use safely (CWE-78),"Specify the application path in the first argument, NOT as part of the second, or embedded spaces could allow an attacker to force a different program to run",,CWE-78," CreateProcess(NULL, ""C:\\Program Files\\GoodGuy\\GoodGuy.exe -x"", """");",3c712b38d0857bde3832d85ad35ac9859be55c5f5f1c20af659a577dd4d0acbf -test.c,97,20,3,buffer,getopt_long,"Some older implementations do not protect against internal buffer overflows (CWE-120, CWE-20)","Check implementation on installation, or limit the size of all string inputs",,"CWE-120, CWE-20"," while ((optc = getopt_long (argc, argv, ""a"",longopts, NULL )) != EOF) {",5bedf6e5bccf596008ef191ec4c5d4cc51a32cff0c05ef62d5f10fab93d0cc24 +test.c,81,10,3,misc,LoadLibraryEx,"Ensure that the full path to the library is specified, or current directory may be used (CWE-829, CWE-20)",Use a flag like LOAD_LIBRARY_SEARCH_SYSTEM32 or LOAD_LIBRARY_SEARCH_APPLICATION_DIR to search only desired folders,,"CWE-829, CWE-20"," (void) LoadLibraryEx(L""user32.dll"", nullptr, LOAD_LIBRARY_AS_DATAFILE);",b1f99ecaa31e682487d795afbf03282fd56ad9f2aa630d0196219b277d2a68c9 +test.c,99,20,3,buffer,getopt_long,"Some older implementations do not protect against internal buffer overflows (CWE-120, CWE-20)","Check implementation on installation, or limit the size of all string inputs",,"CWE-120, CWE-20"," while ((optc = getopt_long (argc, argv, ""a"",longopts, NULL )) != EOF) {",5bedf6e5bccf596008ef191ec4c5d4cc51a32cff0c05ef62d5f10fab93d0cc24 test.c,16,2,2,buffer,strcpy,Does not check for buffer overflows when copying to destination [MS-banned] (CWE-120),"Consider using snprintf, strcpy_s, or strlcpy (warning: strncpy easily misused)",Risk is low because the source is a constant string.,CWE-120," strcpy(a, gettext(""Hello there"")); // Did this work?",d64070fb93ff0bb797fb926f4dddc7212d42f77e288d5ceb0cd30ed2979fa28d test.c,19,2,2,buffer,sprintf,Does not check for buffer overflows (CWE-120),"Use sprintf_s, snprintf, or vsnprintf",Risk is low because the source has a constant maximum length.,CWE-120," sprintf(s, ""hello"");",907b46be1c3ea7b38f90a4d1b0f43b7751cd8cbe38fae840930ff006b702157d test.c,45,3,2,buffer,char,"Statically-sized arrays can be improperly restricted, leading to potential overflows or other issues (CWE-119!/CWE-120)","Perform bounds checking, use functions that limit length, or ensure that the size is larger than the maximum possible length",,CWE-119!/CWE-120, char d[20];,36c87517700337a59cc3ad3218cfdde56cad37d69cdeccee5a55ab232d5c7946 @@ -27,7 +28,7 @@ test.c,50,3,2,buffer,memcpy,Does not check for buffer overflows when copying to test.c,53,3,2,buffer,memcpy,Does not check for buffer overflows when copying to destination (CWE-120),Make sure destination can always hold the source data,,CWE-120," memcpy(&n,s,sizeof(s)); // fail - sizeof not of destination",01bcc2c8ba2d928ac3315b4dcc6593042ea05e62888a10a6d2cf16797a65ed32 test.c,54,3,2,buffer,memcpy,Does not check for buffer overflows when copying to destination (CWE-120),Make sure destination can always hold the source data,,CWE-120," memcpy(d,s,n); // fail - size unguessable",2517a2fb5981193a6017cca660d16e85aab133706cbec302df97aaa623fc77ef test.c,55,3,2,buffer,CopyMemory,Does not check for buffer overflows when copying to destination (CWE-120),Make sure destination can always hold the source data,,CWE-120," CopyMemory(d,s);",977f8c805ddd76ff32e0f7aea08701ba97d9ce6955136e98b308ed4f70eb2e11 -test.c,103,7,2,misc,fopen,"Check when opening files - can an attacker redirect it (via symlinks), force the opening of special file type (e.g., device files), move things around to create a race condition, control its ancestors, or change its contents? (CWE-362)",,,CWE-362," f = fopen(""/etc/passwd"", ""r""); ",2ec6928c77a8b54caa61d0459f367c4394ee1f5e6f488753f587bfa9c780bad8 +test.c,105,7,2,misc,fopen,"Check when opening files - can an attacker redirect it (via symlinks), force the opening of special file type (e.g., device files), move things around to create a race condition, control its ancestors, or change its contents? (CWE-362)",,,CWE-362," f = fopen(""/etc/passwd"", ""r""); ",2ec6928c77a8b54caa61d0459f367c4394ee1f5e6f488753f587bfa9c780bad8 test.c,15,2,1,buffer,strcpy,Does not check for buffer overflows when copying to destination [MS-banned] (CWE-120),"Consider using snprintf, strcpy_s, or strlcpy (warning: strncpy easily misused)",Risk is low because the source is a constant character.,CWE-120," strcpy(a, ""\n""); // Did this work?",0badc5f4c500d17b42794feaca54ee0f49e607a32510af3ed749579001017edb test.c,18,2,1,buffer,sprintf,Does not check for buffer overflows (CWE-120),"Use sprintf_s, snprintf, or vsnprintf",Risk is low because the source is a constant character.,CWE-120," sprintf(s, ""\n"");",c65fbd60851f3c8ace22332805966606488c0d242c1823493c582e267609b1a7 test.c,26,2,1,buffer,scanf,It's unclear if the %s limit in the format string is small enough (CWE-120),"Check that the limit is sufficiently small, or use a different input function",,CWE-120," scanf(""%10s"", s);",e24c4c801f10acfa93098b2bef58524efe4f88237f2dd8b58be9afa838616afe diff --git a/test/correct-results.html b/test/correct-results.html index 1d8a712..5b53939 100644 --- a/test/correct-results.html +++ b/test/correct-results.html @@ -171,7 +171,17 @@ Examining test2.c 
   CreateProcess(NULL, "C:\\Program Files\\GoodGuy\\GoodGuy.exe -x", "");
-
  • test.c:97: [3] (buffer) getopt_long: +
  • test.c:81: [3] (misc) LoadLibraryEx: + Ensure that the full path to the library is specified, or current directory + may be used (CWE-829, CWE-20). Use a + flag like LOAD_LIBRARY_SEARCH_SYSTEM32 or + LOAD_LIBRARY_SEARCH_APPLICATION_DIR to search only desired folders. +
    +  (void) LoadLibraryEx(L"user32.dll", nullptr, LOAD_LIBRARY_AS_DATAFILE);
+
    +
  • test.c:99: [3] (buffer) getopt_long: Some older implementations do not protect against internal buffer overflows (CWE-120, CWE-20). Check @@ -243,7 +253,7 @@ Examining test2.c 
       CopyMemory(d,s);
    -
  • test.c:103: [2] (misc) fopen: +
  • test.c:105: [2] (misc) fopen: Check when opening files - can an attacker redirect it (via symlinks), force the opening of special file type (e.g., device files), move things around to create a race condition, control its ancestors, or change its @@ -321,15 +331,15 @@ Examining test2.c

    Analysis Summary

    -Hits = 38 +Hits = 39
    -Lines analyzed = 124 +Lines analyzed = 126
    -Physical Source Lines of Code (SLOC) = 85 +Physical Source Lines of Code (SLOC) = 86
    -Hits@level = [0] 16 [1] 9 [2] 9 [3] 3 [4] 10 [5] 7
    -Hits@level+ = [0+] 54 [1+] 38 [2+] 29 [3+] 20 [4+] 17 [5+] 7
    -Hits/KSLOC@level+ = [0+] 635.294 [1+] 447.059 [2+] 341.176 [3+] 235.294 [4+] 200 [5+] 82.3529
    +Hits@level = [0] 16 [1] 9 [2] 9 [3] 4 [4] 10 [5] 7
    +Hits@level+ = [0+] 55 [1+] 39 [2+] 30 [3+] 21 [4+] 17 [5+] 7
    +Hits/KSLOC@level+ = [0+] 639.535 [1+] 453.488 [2+] 348.837 [3+] 244.186 [4+] 197.674 [5+] 81.3953
    Suppressed hits = 2 (use --neverignore to show them)
    Minimum risk level = 1 diff --git a/test/correct-results.txt b/test/correct-results.txt index b7f69ea1c63ab234cf57f7bd6cfdb83f13441343..84e1963374ac7e2790b76ed042d4c487f4f8ab02 100644 GIT binary patch literal 8952 zcmeHNZExE+68`RAG5eutvUME4#d#?fcU~up2IssWPEi!SML+cy- zlI5hm4X}IH_HI8#vBlwVIP=UiGyWwJ4-2JZnbW<@tumVWoo=T$Y}3wUm-^jqZ?E6& z9n-0}S22w{^ll|(BJ)n`vPkDxW)_qeiL|tnPW0%5K0ayva(+3U(#_fI?eunb(rSe_PX_%H zq7MhZ($2hCEM&e*OX=)Mi|EuqDQz4@D;a&Hg~=(rLIy`&B<2A+Pu`vF_4?g5@k776 z+o88s(t>5|u(sAo5qGN1M%~AoS(ocbINt9}SU!7UWsHT9$#WSwe6S-s%T9aiqsXLL z;rw}5lFC;p6`xdEq!4A*KP5RsB93?I!^LcGF0__04q568Jtsz6#rFmkpuwGO^HYLH zJZRj7c8fM-;n<>ZCKXOaB7r`+gm~T?uj7yH(2cSmNm*K{R4Ums(UNvfrC!rGNg!q%Mbg^cHa)CVw4w)7Br!p>W^kMX z-bKtX3|tV{oP>WpV9w#7zgyqQ_pE!*+kM!`7mT0T`neh7VFhzVCR=;-HWVXaDGhK% zj%y$_to=ExF02RM3Z6Z7XKTi8*xSQ*hJ^7Ta!!UP{E3Ec5GF%%fv<3W*<3~6XQZv; z1n7V6c^X=B=Cfh7&nC5&9{TR9q*A*<-$VCT+S{jn-W#rq+_qb3&gZ^1PHLw_!hVe} z%S=aEZsv(h%bOVtfoK=vBhvlWTUBD)oO8gT+T3en;`7D!YG4@R5D8N0MN&u|feg&o z%|LB4O0KKdJS1*P;O-a(Sj0{0z+(bk2c{&e9GDTIUH){7vtEaFaXkv z;N$RK5|9!rCTqAQ7z$LV=82orJdM-u_%wr~>Tbdn!d=%kF-yK2xKQ{9cT!z&U}lw* zJkOUPs`jR)i$C?HMo6${h23k*505KdD?hqR=k~u0k)rm9x-Rhy!#a~8#!CDxDTmY? zKA@jWU4zo~X%0MpXo6J2fFCIPb4r8~@R^ZH#(6GMwqPGyH-NtZ`&HF&=nXmmR@s$I zl3kkkM1GwcPRAiWL6{E}{g-kd{ zcql(S0;*N<$MyhkPfDl&p9iT2(c{k-Dz^ytd|9Lj!!})B-7*CO<#!unS_w3aeRh(l z_Tm_DW-|fRa0kpDP*IqNCcLp$aH{BWnBzc86chY)UTi??Pul!^(WeNqC#xro?j@E)zu4gKvyVAOhE8_esT? z6Bk@64z6_6KYmmfBPtWIgjgia>DBo3ZhHRn&G_d1-Rx|9GkJ42dq2B9yBPG@f%4D3 zF}}W@o=?WN=U10^r{_294i;r4*r?ImI6!ThWB-LoP}JGmJOXD z232s9lvxy~4(XGvU)D9iCA?5^5TEWg2kUuw>VNNaw5|*p4d&8AnfE|z3dRCBV1v#8 zfB~cR<@WsLZrNZ%u$}|a)pY^+IN3di!~T}SD?njGwsX{vj=0I-*k(r>`R3@0L;;xU8A`LLQ8W~=omY)<3ebgBOAi1`Wv~0aU-~E==TJie*`L3J)a;|U}G%u`2&(F zqh!Wf1!DcgI!}E2PwT8|-#PG=Thn(^nf^g6YZDN-CST(CMA58Oxl9qs1N-C(xY-QW zSU9m!(-%MAzM8)M_3f+q1TZF`vKKN%3TD`N4%+>@4#XdL@x5-XYoH$s0G4JYLn`!Y z@J9?-3&j(FJzub}V^vKl;tp;1$gG}st8r_LiTL~3`HW_7FD_7qV=DfJv)fNJIBrdqMoJc_aMxUa{6xL}sCB(s zTP{mbFZ;SIyo9VfF)h&Q?Kji@I;4=#jpJ8S9tB<0L%Gd!0Y`377X~I--85x=p8}?Ka?`||Hc{xHEKzt z!ExtcP)Zvf3_8Q3qablGJn9?`AcLj$hr>?q2s*Rm-tl2)bO=RR`cbblI6fG(W<{1^ z5X$|IRWMthb{LNL_B0RN)KVL6ub@S-LRkx*lJ2%HFd)N_o^qabh4Lsq04*}j&&i3fONHm)6bCqDy*J-^sg(po)uhbk1L0e>Ia_5Hzu`dR>3{AP8 z5juAe&E?%mYk!}+G3Bo}PAF*G@@)S{ru#o4s*%{xvk?gng(?k1=wAL%@5x6E43P6p*;>^YN?Uity2r0dWML=sxko2z%X zS1(&TE9WwMvcDgLR{Ecp;LQ7e1Gxu;;npF3J%WRS_dySI#C<~c^N-f8pTU8J;%kkD UDJUOAqm2rxDNmk80vc5R0&yj{+yDRo 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 From 49fd4b2ec97e19c68e66ff06ad115a88296ea301 Mon Sep 17 00:00:00 2001 From: Charles Milette Date: Sun, 10 Jan 2021 18:01:03 -0500 Subject: [PATCH 3/3] Move safe_search to globals and add LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR to the list of safe flags --- flawfinder | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/flawfinder b/flawfinder index 89f29e8..5957afe 100755 --- a/flawfinder +++ b/flawfinder @@ -846,24 +846,27 @@ def cpp_unsafe_stl(hit): if len(hit.parameters) <= 4: add_warning(hit) +safe_load_library_flags = [ + # 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', + # This flag requires an absolute path to the DLL to be passed + 'LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR', + # 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' +] + def load_library_ex(hit): # 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 - any(flag in hit.parameters[3] for flag in safe_search)): + any(flag in hit.parameters[3] for flag in safe_load_library_flags)): return normal(hit)