Examining test.c
Examining test2.c
@@ -28,17 +28,17 @@ Examining test2.c
strncat(d,s,sizeof(d)); /* Misuse - this should be flagged as riskier. */
strcpy(b, a); @@ -147,7 +147,8 @@ Examining test2.c
_mbscpy(d,s); /* like strcpy, this doesn't check for buffer overflow */
lstrcat(d,s); @@ -181,7 +182,7 @@ Examining test2.c
strcpy(a, gettext("Hello there")); // Did this work?
@@ -238,7 +239,7 @@ Examining test2.c
strcpy(a, "\n"); // Did this work? @@ -274,9 +275,9 @@ Examining test2.c
strncat(d,s,10);diff --git a/correct-results.txt b/correct-results.txt index 40d3574..b8bcb20 100644 --- a/correct-results.txt +++ b/correct-results.txt @@ -1,5 +1,5 @@ Flawfinder version 1.32, (C) 2001-2014 David A. Wheeler. -Number of rules (primarily dangerous function names) in C/C++ ruleset: 188 +Number of rules (primarily dangerous function names) in C/C++ ruleset: 209 Examining test.c Examining test2.c @@ -9,14 +9,15 @@ test.c:32: [5] (buffer) gets: Does not check for buffer overflows (CWE-120, CWE-20). Use fgets() instead. test.c:56: [5] (buffer) strncat: Easily used incorrectly (e.g., incorrectly computing the correct maximum - size to add) (CWE-120). Consider strcat_s, strlcat, or automatically - resizing strings. Risk is high; the length parameter appears to be a - constant, instead of computing the number of characters left. + size to add) [MS-banned] (CWE-120). Consider strcat_s, strlcat, snprintf, + or automatically resizing strings. Risk is high; the length parameter + appears to be a constant, instead of computing the number of characters + left. test.c:57: [5] (buffer) _tcsncat: Easily used incorrectly (e.g., incorrectly computing the correct maximum - size to add) (CWE-120). Consider strcat_s, strlcat, or automatically - resizing strings. Risk is high; the length parameter appears to be a - constant, instead of computing the number of characters left. + size to add) [MS-banned] (CWE-120). Consider strcat_s, strlcat, or + automatically resizing strings. Risk is high; the length parameter appears + to be a constant, instead of computing the number of characters left. test.c:60: [5] (buffer) MultiByteToWideChar: Requires maximum length in CHARACTERS, not bytes (CWE-120). Risk is high, it appears that the size is given as bytes, but the function requires size @@ -33,8 +34,8 @@ test.c:73: [5] (misc) SetSecurityDescriptorDacl: Access), which would even forbid administrator access (CWE-732). test.c:17: [4] (buffer) strcpy: Does not check for buffer overflows when copying to destination [MS-banned] - (CWE-120). Consider using strcpy_s, strncpy, or strlcpy (warning, strncpy - is easily misused). + (CWE-120). Consider using snprintf, strcpy_s, or strlcpy (warning: strncpy + easily misused). test.c:20: [4] (buffer) sprintf: Does not check for buffer overflows (CWE-120). Use sprintf_s, snprintf, or vsnprintf. @@ -63,7 +64,7 @@ test.c:49: [4] (buffer) _mbscpy: of the buffer. test.c:52: [4] (buffer) lstrcat: Does not check for buffer overflows when concatenating to destination - (CWE-120). + [MS-banned] (CWE-120). test.c:75: [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 @@ -80,8 +81,8 @@ test.c:91: [3] (buffer) getopt_long: of all string inputs. test.c:16: [2] (buffer) strcpy: Does not check for buffer overflows when copying to destination [MS-banned] - (CWE-120). Consider using strcpy_s, strncpy, or strlcpy (warning, strncpy - is easily misused). Risk is low because the source is a constant string. + (CWE-120). Consider using snprintf, strcpy_s, or strlcpy (warning: strncpy + easily misused). Risk is low because the source is a constant string. test.c:19: [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. @@ -108,8 +109,8 @@ test.c:97: [2] (misc) fopen: contents? (CWE-362). test.c:15: [1] (buffer) strcpy: Does not check for buffer overflows when copying to destination [MS-banned] - (CWE-120). Consider using strcpy_s, strncpy, or strlcpy (warning, strncpy - is easily misused). Risk is low because the source is a constant character. + (CWE-120). Consider using snprintf, strcpy_s, or strlcpy (warning: strncpy + easily misused). Risk is low because the source is a constant character. test.c:18: [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. @@ -125,8 +126,8 @@ test.c:54: [1] (buffer) _tcsncpy: pointers [MS-banned] (CWE-120). test.c:55: [1] (buffer) strncat: Easily used incorrectly (e.g., incorrectly computing the correct maximum - size to add) (CWE-120). Consider strcat_s, strlcat, or automatically - resizing strings. + size to add) [MS-banned] (CWE-120). Consider strcat_s, strlcat, snprintf, + or automatically resizing strings. test.c:58: [1] (buffer) strlen: Does not handle strings that are not \0-terminated; if given one it may perform an over-read (it could cause a crash if unprotected) (CWE-126). diff --git a/flawfinder b/flawfinder index 405db48..c8decca 100755 --- a/flawfinder +++ b/flawfinder @@ -597,18 +597,13 @@ def c_printf(hit): source = strip_i18n(hit.parameters[format_position]) if c_constant_string(source): # Parameter is constant, so there's no risk of format string problems. - if hit.name == "snprintf" or hit.name == "vsnprintf": - hit.level = 1 - hit.warning = \ - "On some very old systems, snprintf is incorrectly implemented " \ - "and permits buffer overflows; there are also incompatible " \ - "standard definitions of it" - hit.suggestion = "Check it during installation, or use something else" - hit.category = "port" - else: - # We'll pass it on, just in case it's needed, but at level 0 risk. - hit.level = 0 - hit.note = "Constant format string, so not considered very risky (there's some residual risk, especially in a loop)." + # At one time we warned that very old systems sometimes incorrectly + # allow buffer overflows on snprintf/vsnprintf, but those systems + # are now very old, and snprintf is an important potential tool for + # countering buffer overflows. + # We'll pass it on, just in case it's needed, but at level 0 risk. + hit.level = 0 + hit.note = "Constant format string, so not considered risky." add_warning(hit) @@ -740,7 +735,7 @@ c_ruleset = { "strcpy" : (c_buffer, 4, "Does not check for buffer overflows when copying to destination [MS-banned] (CWE-120)", - "Consider using strcpy_s, strncpy, or strlcpy (warning, strncpy is easily misused)", + "Consider using snprintf, strcpy_s, or strlcpy (warning: strncpy easily misused)", "buffer", "", {}), "strcpyA|strcpyW|StrCpy|StrCpyA|lstrcpyA|lstrcpyW|_tccpy|_mbccpy|_ftcscpy|_mbsncpy|StrCpyN|StrCpyNA|StrCpyNW|StrNCpy|strcpynA|StrNCpyA|StrNCpyW|lstrcpynA|lstrcpynW" : # We need more info on these functions; I got their names from the @@ -748,7 +743,7 @@ c_ruleset = { # instead of "c_buffer". (normal, 4, "Does not check for buffer overflows when copying to destination [MS-banned] (CWE-120)", - "Consider using strcpy_s, strncpy, or strlcpy (warning, strncpy is easily misused)", + "Consider using snprintf, strcpy_s, or strlcpy (warning: strncpy easily misused)", "buffer", "", {}), "lstrcpy|wcscpy|_tcscpy|_mbscpy" : (c_buffer, 4, @@ -763,7 +758,7 @@ c_ruleset = { "strcat" : (c_buffer, 4, "Does not check for buffer overflows when concatenating to destination [MS-banned] (CWE-120)", - "Consider using strcat_s, strncat, or strlcat (warning, strncat is easily misused)", + "Consider using strcat_s, strncat, strlcat, or snprintf (warning: strncat is easily misused)", "buffer", "", {}), "lstrcat|wcscat|_tcscat|_mbscat" : (c_buffer, 4, @@ -798,7 +793,7 @@ c_ruleset = { # FIXING security problems, and raising it to a # higher risk level would cause many false positives. "Easily used incorrectly (e.g., incorrectly computing the correct maximum size to add) [MS-banned] (CWE-120)", - "Consider strcat_s, strlcat, or automatically resizing strings", + "Consider strcat_s, strlcat, snprintf, or automatically resizing strings", "buffer", "", {}), "lstrcatn|wcsncat|_tcsncat|_mbsnbcat" : (c_strncat, @@ -874,8 +869,8 @@ c_ruleset = { "strlen|wcslen|_tcslen|_mbslen" : (normal, - 1, # Often this isn't really a risk, and even when, it usually at worst causes - # program crash (and nothing worse). + 1, # Often this isn't really a risk, and even when it is, at worst it + # often causes a program crash (and nothing worse). "Does not handle strings that are not \\0-terminated; " + "if given one it may perform an over-read (it could cause a crash " + "if unprotected) (CWE-126)",