Ignore LoadLibraryEx if LOAD_LIBRARY_SEARCH_SYSTEM32

Ignore LoadLibraryEx if its third parameter is
LOAD_LIBRARY_SEARCH_SYSTEM32, as this is safe.
This eliminates a false positive.

See:
https://github.com/david-a-wheeler/flawfinder/issues/26

Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>

flawfinder

@@ -846,6 +846,12 @@ def cpp_unsafe_stl(hit):
     if len(hit.parameters) <= 4:
         add_warning(hit)
 
+def load_library_ex(hit):
+    # If parameter 3 is 'LOAD_LIBRARY_SEARCH_SYSTEM32', it's safe.
+    if (len(hit.parameters) >= 4 and
+       hit.parameters[3] == 'LOAD_LIBRARY_SEARCH_SYSTEM32'):
+        return
+    normal(hit)
 
 def normal(hit):
     add_warning(hit)
@@ -1290,11 +1296,16 @@ c_ruleset = {
     #  "Use InitializeCriticalSectionAndSpinCount instead",
     #  "misc", "", {}),
 
-    "LoadLibrary|LoadLibraryEx":
+    "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",
      "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",
+     "misc", "", {'input': 1}),
+
     "SetSecurityDescriptorDacl":
     (c_hit_if_null, 5,
      "Never create NULL ACLs; an attacker can set it to Everyone (Deny All Access), "

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 = 121
+Lines analyzed = 123
-Physical Source Lines of Code (SLOC) = 84
+Physical Source Lines of Code (SLOC) = 85
 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

test/correct-results.csv

@@ -18,7 +18,7 @@ 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,95,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,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,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 +27,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,101,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,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,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

test/correct-results.html

@@ -171,7 +171,7 @@ Examining test2.c <br>
 <pre>
   CreateProcess(NULL, "C:\\Program Files\\GoodGuy\\GoodGuy.exe -x", "");
 </pre>
-<li>test.c:95: <b>  [3] </b> (buffer) <i> getopt_long:
+<li>test.c:97: <b>  [3] </b> (buffer) <i> getopt_long:
   Some older implementations do not protect against internal buffer overflows
   (<a href="https://cwe.mitre.org/data/definitions/120.html">CWE-120</a>, <a
   href="https://cwe.mitre.org/data/definitions/20.html">CWE-20</a>). Check
@@ -243,7 +243,7 @@ Examining test2.c <br>
 <pre>
   CopyMemory(d,s);
 </pre>
-<li>test.c:101: <b>  [2] </b> (misc) <i> fopen:
+<li>test.c:103: <b>  [2] </b> (misc) <i> 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
@@ -323,13 +323,13 @@ Examining test2.c <br>
 <p>
 Hits = 38
 <br>
-Lines analyzed = 122
+Lines analyzed = 124
 <br>
-Physical Source Lines of Code (SLOC) = 84
+Physical Source Lines of Code (SLOC) = 85
 <br>
 Hits@level = [0]  16 [1]   9 [2]   9 [3]   3 [4]  10 [5]   7 <br>
 Hits@level+ = [0+]  54 [1+]  38 [2+]  29 [3+]  20 [4+]  17 [5+]   7 <br>
-Hits/KSLOC@level+ = [0+] 642.857 [1+] 452.381 [2+] 345.238 [3+] 238.095 [4+] 202.381 [5+] 83.3333 <br>
+Hits/KSLOC@level+ = [0+] 635.294 [1+] 447.059 [2+] 341.176 [3+] 235.294 [4+] 200 [5+] 82.3529 <br>
 Suppressed hits = 2 (use --neverignore to show them)
 <br>
 Minimum risk level = 1

test/correct-results.txt

@@ -75,7 +75,7 @@ test.c:79:  [3] (shell) CreateProcess:
   (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.
-test.c:95:  [3] (buffer) getopt_long:
+test.c:97:  [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.
@@ -108,7 +108,7 @@ test.c:54:  [2] (buffer) memcpy:
 test.c:55:  [2] (buffer) CopyMemory:
   Does not check for buffer overflows when copying to destination (CWE-120).
   Make sure destination can always hold the source data.
-test.c:101:  [2] (misc) fopen:
+test.c:103:  [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
@@ -147,11 +147,11 @@ test.c:70:  [1] (buffer) MultiByteToWideChar:
 ANALYSIS SUMMARY:
 
 Hits = 38
-Lines analyzed = 122
+Lines analyzed = 124
-Physical Source Lines of Code (SLOC) = 84
+Physical Source Lines of Code (SLOC) = 85
 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+] 642.857 [1+] 452.381 [2+] 345.238 [3+] 238.095 [4+] 202.381 [5+] 83.3333
+Hits/KSLOC@level+ = [0+] 635.294 [1+] 447.059 [2+] 341.176 [3+] 235.294 [4+] 200 [5+] 82.3529
 Suppressed hits = 2 (use --neverignore to show them)
 Minimum risk level = 1
 

test/test.c

@@ -77,6 +77,8 @@ 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);
   /* Test interaction of quote characters */
   printf("%c\n", 'x');
   printf("%c\n", '"');