Add Microsoft banned functions for string copy and replacement

This commit is contained in:
David A. Wheeler 2014-08-09 13:32:37 -04:00
parent 0b432d2791
commit aa2277b862
4 changed files with 51 additions and 36 deletions

View File

@ -9,9 +9,9 @@
<body> <body>
<h1>Flawfinder Results</h1> <h1>Flawfinder Results</h1>
Here are the security scan results from Here are the security scan results from
<a href="http://www.dwheeler.com/flawfinder">Flawfinder version 1.31</a>, <a href="http://www.dwheeler.com/flawfinder">Flawfinder version 1.32</a>,
(C) 2001-2014 <a href="http://www.dwheeler.com">David A. Wheeler</a>. (C) 2001-2014 <a href="http://www.dwheeler.com">David A. Wheeler</a>.
Number of rules (primarily dangerous function names) in C/C++ ruleset: 169 Number of rules (primarily dangerous function names) in C/C++ ruleset: 188
<p> <p>
Examining test.c <br> Examining test.c <br>
Examining test2.c <br> Examining test2.c <br>
@ -77,8 +77,8 @@ Examining test2.c <br>
SetSecurityDescriptorDacl(&amp;sd,TRUE,NULL,FALSE); SetSecurityDescriptorDacl(&amp;sd,TRUE,NULL,FALSE);
</pre> </pre>
<li>test.c:17: <b> [4] </b> (buffer) <i> strcpy: <li>test.c:17: <b> [4] </b> (buffer) <i> strcpy:
Does not check for buffer overflows when copying to destination (<a Does not check for buffer overflows when copying to destination [MS-banned]
href="http://cwe.mitre.org/data/definitions/120.html">CWE-120</a>). (<a href="http://cwe.mitre.org/data/definitions/120.html">CWE-120</a>).
Consider using strcpy_s, strncpy, or strlcpy (warning, strncpy is easily Consider using strcpy_s, strncpy, or strlcpy (warning, strncpy is easily
misused). </i> misused). </i>
<pre> <pre>
@ -139,8 +139,8 @@ Examining test2.c <br>
syslog(LOG_ERR, attacker_string); syslog(LOG_ERR, attacker_string);
</pre> </pre>
<li>test.c:49: <b> [4] </b> (buffer) <i> _mbscpy: <li>test.c:49: <b> [4] </b> (buffer) <i> _mbscpy:
Does not check for buffer overflows when copying to destination (<a Does not check for buffer overflows when copying to destination [MS-banned]
href="http://cwe.mitre.org/data/definitions/120.html">CWE-120</a>). (<a href="http://cwe.mitre.org/data/definitions/120.html">CWE-120</a>).
Consider using a function version that stops copying at the end of the Consider using a function version that stops copying at the end of the
buffer. </i> buffer. </i>
<pre> <pre>
@ -179,8 +179,8 @@ Examining test2.c <br>
while ((optc = getopt_long (argc, argv, "a",longopts, NULL )) != EOF) { while ((optc = getopt_long (argc, argv, "a",longopts, NULL )) != EOF) {
</pre> </pre>
<li>test.c:16: <b> [2] </b> (buffer) <i> strcpy: <li>test.c:16: <b> [2] </b> (buffer) <i> strcpy:
Does not check for buffer overflows when copying to destination (<a Does not check for buffer overflows when copying to destination [MS-banned]
href="http://cwe.mitre.org/data/definitions/120.html">CWE-120</a>). (<a href="http://cwe.mitre.org/data/definitions/120.html">CWE-120</a>).
Consider using strcpy_s, strncpy, or strlcpy (warning, strncpy is easily Consider using strcpy_s, strncpy, or strlcpy (warning, strncpy is easily
misused). Risk is low because the source is a constant string. </i> misused). Risk is low because the source is a constant string. </i>
<pre> <pre>
@ -196,7 +196,7 @@ Examining test2.c <br>
</pre> </pre>
<li>test.c:45: <b> [2] </b> (buffer) <i> char: <li>test.c:45: <b> [2] </b> (buffer) <i> char:
Statically-sized arrays can be improperly restricted, leading to potential Statically-sized arrays can be improperly restricted, leading to potential
overflows or other issues (CWE-119:<a overflows or other issues (CWE-119!/<a
href="http://cwe.mitre.org/data/definitions/120.html">CWE-120</a>). Perform href="http://cwe.mitre.org/data/definitions/120.html">CWE-120</a>). Perform
bounds checking, use functions that limit length, or ensure that the size bounds checking, use functions that limit length, or ensure that the size
is larger than the maximum possible length. </i> is larger than the maximum possible length. </i>
@ -205,7 +205,7 @@ Examining test2.c <br>
</pre> </pre>
<li>test.c:46: <b> [2] </b> (buffer) <i> char: <li>test.c:46: <b> [2] </b> (buffer) <i> char:
Statically-sized arrays can be improperly restricted, leading to potential Statically-sized arrays can be improperly restricted, leading to potential
overflows or other issues (CWE-119:<a overflows or other issues (CWE-119!/<a
href="http://cwe.mitre.org/data/definitions/120.html">CWE-120</a>). Perform href="http://cwe.mitre.org/data/definitions/120.html">CWE-120</a>). Perform
bounds checking, use functions that limit length, or ensure that the size bounds checking, use functions that limit length, or ensure that the size
is larger than the maximum possible length. </i> is larger than the maximum possible length. </i>
@ -236,8 +236,8 @@ Examining test2.c <br>
f = fopen("/etc/passwd", "r"); f = fopen("/etc/passwd", "r");
</pre> </pre>
<li>test.c:15: <b> [1] </b> (buffer) <i> strcpy: <li>test.c:15: <b> [1] </b> (buffer) <i> strcpy:
Does not check for buffer overflows when copying to destination (<a Does not check for buffer overflows when copying to destination [MS-banned]
href="http://cwe.mitre.org/data/definitions/120.html">CWE-120</a>). (<a href="http://cwe.mitre.org/data/definitions/120.html">CWE-120</a>).
Consider using strcpy_s, strncpy, or strlcpy (warning, strncpy is easily Consider using strcpy_s, strncpy, or strlcpy (warning, strncpy is easily
misused). Risk is low because the source is a constant character. </i> misused). Risk is low because the source is a constant character. </i>
<pre> <pre>
@ -260,14 +260,14 @@ Examining test2.c <br>
</pre> </pre>
<li>test.c:53: <b> [1] </b> (buffer) <i> strncpy: <li>test.c:53: <b> [1] </b> (buffer) <i> strncpy:
Easily used incorrectly; doesn't always \0-terminate or check for invalid Easily used incorrectly; doesn't always \0-terminate or check for invalid
pointers (<a pointers [MS-banned] (<a
href="http://cwe.mitre.org/data/definitions/120.html">CWE-120</a>). </i> href="http://cwe.mitre.org/data/definitions/120.html">CWE-120</a>). </i>
<pre> <pre>
strncpy(d,s); strncpy(d,s);
</pre> </pre>
<li>test.c:54: <b> [1] </b> (buffer) <i> _tcsncpy: <li>test.c:54: <b> [1] </b> (buffer) <i> _tcsncpy:
Easily used incorrectly; doesn't always \0-terminate or check for invalid Easily used incorrectly; doesn't always \0-terminate or check for invalid
pointers (<a pointers [MS-banned] (<a
href="http://cwe.mitre.org/data/definitions/120.html">CWE-120</a>). </i> href="http://cwe.mitre.org/data/definitions/120.html">CWE-120</a>). </i>
<pre> <pre>
_tcsncpy(d,s); _tcsncpy(d,s);

View File

@ -1,5 +1,5 @@
Flawfinder version 1.31, (C) 2001-2014 David A. Wheeler. Flawfinder version 1.32, (C) 2001-2014 David A. Wheeler.
Number of rules (primarily dangerous function names) in C/C++ ruleset: 169 Number of rules (primarily dangerous function names) in C/C++ ruleset: 188
Examining test.c Examining test.c
Examining test2.c Examining test2.c
@ -32,9 +32,9 @@ test.c:73: [5] (misc) SetSecurityDescriptorDacl:
Never create NULL ACLs; an attacker can set it to Everyone (Deny All Never create NULL ACLs; an attacker can set it to Everyone (Deny All
Access), which would even forbid administrator access (CWE-732). Access), which would even forbid administrator access (CWE-732).
test.c:17: [4] (buffer) strcpy: test.c:17: [4] (buffer) strcpy:
Does not check for buffer overflows when copying to destination (CWE-120). Does not check for buffer overflows when copying to destination [MS-banned]
Consider using strcpy_s, strncpy, or strlcpy (warning, strncpy is easily (CWE-120). Consider using strcpy_s, strncpy, or strlcpy (warning, strncpy
misused). is easily misused).
test.c:20: [4] (buffer) sprintf: test.c:20: [4] (buffer) sprintf:
Does not check for buffer overflows (CWE-120). Use sprintf_s, snprintf, or Does not check for buffer overflows (CWE-120). Use sprintf_s, snprintf, or
vsnprintf. vsnprintf.
@ -58,9 +58,9 @@ test.c:38: [4] (format) syslog:
If syslog's format strings can be influenced by an attacker, they can be If syslog's format strings can be influenced by an attacker, they can be
exploited (CWE-134). Use a constant format string for syslog. exploited (CWE-134). Use a constant format string for syslog.
test.c:49: [4] (buffer) _mbscpy: test.c:49: [4] (buffer) _mbscpy:
Does not check for buffer overflows when copying to destination (CWE-120). Does not check for buffer overflows when copying to destination [MS-banned]
Consider using a function version that stops copying at the end of the (CWE-120). Consider using a function version that stops copying at the end
buffer. of the buffer.
test.c:52: [4] (buffer) lstrcat: test.c:52: [4] (buffer) lstrcat:
Does not check for buffer overflows when concatenating to destination Does not check for buffer overflows when concatenating to destination
(CWE-120). (CWE-120).
@ -79,20 +79,20 @@ test.c:91: [3] (buffer) getopt_long:
(CWE-120, CWE-20). Check implementation on installation, or limit the size (CWE-120, CWE-20). Check implementation on installation, or limit the size
of all string inputs. of all string inputs.
test.c:16: [2] (buffer) strcpy: test.c:16: [2] (buffer) strcpy:
Does not check for buffer overflows when copying to destination (CWE-120). Does not check for buffer overflows when copying to destination [MS-banned]
Consider using strcpy_s, strncpy, or strlcpy (warning, strncpy is easily (CWE-120). Consider using strcpy_s, strncpy, or strlcpy (warning, strncpy
misused). Risk is low because the source is a constant string. is easily misused). Risk is low because the source is a constant string.
test.c:19: [2] (buffer) sprintf: test.c:19: [2] (buffer) sprintf:
Does not check for buffer overflows (CWE-120). Use sprintf_s, snprintf, or 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. vsnprintf. Risk is low because the source has a constant maximum length.
test.c:45: [2] (buffer) char: test.c:45: [2] (buffer) char:
Statically-sized arrays can be improperly restricted, leading to potential Statically-sized arrays can be improperly restricted, leading to potential
overflows or other issues (CWE-119:CWE-120). Perform bounds checking, use 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 functions that limit length, or ensure that the size is larger than the
maximum possible length. maximum possible length.
test.c:46: [2] (buffer) char: test.c:46: [2] (buffer) char:
Statically-sized arrays can be improperly restricted, leading to potential Statically-sized arrays can be improperly restricted, leading to potential
overflows or other issues (CWE-119:CWE-120). Perform bounds checking, use 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 functions that limit length, or ensure that the size is larger than the
maximum possible length. maximum possible length.
test.c:50: [2] (buffer) memcpy: test.c:50: [2] (buffer) memcpy:
@ -107,9 +107,9 @@ test.c:97: [2] (misc) fopen:
around to create a race condition, control its ancestors, or change its around to create a race condition, control its ancestors, or change its
contents? (CWE-362). contents? (CWE-362).
test.c:15: [1] (buffer) strcpy: test.c:15: [1] (buffer) strcpy:
Does not check for buffer overflows when copying to destination (CWE-120). Does not check for buffer overflows when copying to destination [MS-banned]
Consider using strcpy_s, strncpy, or strlcpy (warning, strncpy is easily (CWE-120). Consider using strcpy_s, strncpy, or strlcpy (warning, strncpy
misused). Risk is low because the source is a constant character. is easily misused). Risk is low because the source is a constant character.
test.c:18: [1] (buffer) sprintf: test.c:18: [1] (buffer) sprintf:
Does not check for buffer overflows (CWE-120). Use sprintf_s, snprintf, or Does not check for buffer overflows (CWE-120). Use sprintf_s, snprintf, or
vsnprintf. Risk is low because the source is a constant character. vsnprintf. Risk is low because the source is a constant character.
@ -119,10 +119,10 @@ test.c:26: [1] (buffer) scanf:
input function. input function.
test.c:53: [1] (buffer) strncpy: test.c:53: [1] (buffer) strncpy:
Easily used incorrectly; doesn't always \0-terminate or check for invalid Easily used incorrectly; doesn't always \0-terminate or check for invalid
pointers (CWE-120). pointers [MS-banned] (CWE-120).
test.c:54: [1] (buffer) _tcsncpy: test.c:54: [1] (buffer) _tcsncpy:
Easily used incorrectly; doesn't always \0-terminate or check for invalid Easily used incorrectly; doesn't always \0-terminate or check for invalid
pointers (CWE-120). pointers [MS-banned] (CWE-120).
test.c:55: [1] (buffer) strncat: test.c:55: [1] (buffer) strncat:
Easily used incorrectly (e.g., incorrectly computing the correct maximum Easily used incorrectly (e.g., incorrectly computing the correct maximum
size to add) (CWE-120). Consider strcat_s, strlcat, or automatically size to add) (CWE-120). Consider strcat_s, strlcat, or automatically

View File

@ -733,15 +733,26 @@ def normal(hit):
# See the definition for class "Hit". # See the definition for class "Hit".
# The key can have multiple values separated with "|". # The key can have multiple values separated with "|".
# For more information on Microsoft banned functions, see:
# http://msdn.microsoft.com/en-us/library/bb288454.aspx
c_ruleset = { c_ruleset = {
"strcpy" : "strcpy" :
(c_buffer, 4, (c_buffer, 4,
"Does not check for buffer overflows when copying to destination (CWE-120)", "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)",
"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
# Microsoft "banned" list. For now, just use "normal" to process them
# 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 strcpy_s, strncpy, or strlcpy (warning, strncpy is easily misused)",
"buffer", "", {}), "buffer", "", {}),
"lstrcpy|wcscpy|_tcscpy|_mbscpy" : "lstrcpy|wcscpy|_tcscpy|_mbscpy" :
(c_buffer, 4, (c_buffer, 4,
"Does not check for buffer overflows when copying to destination (CWE-120)", "Does not check for buffer overflows when copying to destination [MS-banned] (CWE-120)",
"Consider using a function version that stops copying at the end of the buffer", "Consider using a function version that stops copying at the end of the buffer",
"buffer", "", {}), "buffer", "", {}),
"memcpy|CopyMemory|bcopy" : "memcpy|CopyMemory|bcopy" :
@ -764,7 +775,7 @@ c_ruleset = {
1, # Low risk level, because this is often used correctly when FIXING security 1, # Low risk level, because this is often used correctly when FIXING security
# problems, and raising it to a higher risk level would cause many false positives. # problems, and raising it to a higher risk level would cause many false positives.
"Easily used incorrectly; doesn't always \\0-terminate or " + "Easily used incorrectly; doesn't always \\0-terminate or " +
"check for invalid pointers (CWE-120)", "check for invalid pointers [MS-banned] (CWE-120)",
"", "",
"buffer", "", {}), "buffer", "", {}),
"lstrcpyn|wcsncpy|_tcsncpy|_mbsnbcpy" : "lstrcpyn|wcsncpy|_tcsncpy|_mbsnbcpy" :
@ -772,7 +783,7 @@ c_ruleset = {
1, # Low risk level, because this is often used correctly when FIXING security 1, # Low risk level, because this is often used correctly when FIXING security
# problems, and raising it to a higher risk levle would cause many false positives. # problems, and raising it to a higher risk levle would cause many false positives.
"Easily used incorrectly; doesn't always \\0-terminate or " + "Easily used incorrectly; doesn't always \\0-terminate or " +
"check for invalid pointers (CWE-120)", "check for invalid pointers [MS-banned] (CWE-120)",
"", "",
"buffer", "", {}), "buffer", "", {}),
"strncat" : "strncat" :

View File

@ -100,6 +100,10 @@ Hit descriptions also note the relevant
Common Weakness Enumeration (CWE) identifier(s) in parentheses, Common Weakness Enumeration (CWE) identifier(s) in parentheses,
as discussed below. as discussed below.
Flawfinder is officially CWE-Compatible. Flawfinder is officially CWE-Compatible.
Hit descriptions with "[MS-banned]" indicate functions that are in the
banned list of functions released by Microsoft; see
http://msdn.microsoft.com/en-us/library/bb288454.aspx
for more information about banned functions.
.PP .PP
Not every hit is actually a security vulnerability, Not every hit is actually a security vulnerability,
and not every security vulnerability is necessarily found. and not every security vulnerability is necessarily found.