Flawfinderを試す。〜セキュアコーディングを目指して〜
はじめに
コーディング中に、バッファオーバフローをどうやって未然に防ぐか。
Flawfinderというツールがあるとわかったので、試してみた。
インストール
sudo apt-get install flawfinder
サンプルソース
andre@andre-VirtualBox:~/work/flawfinder$ cat -n test.c
1 /* Test flawfinder. This program won't compile or run; that's not necessary
2 for this to be a useful test. */
3
4 #include <stdio.h>
5 #define hello(x) goodbye(x)
6 #define WOKKA "stuff"
7
8 main() {
9 printf("hello\n");
10 }
11
12 /* This is a strcpy test. */
13
14 int demo(char *a, char *b) {
15 strcpy(a, "\n"); // Did this work?
16 strcpy(a, gettext("Hello there")); // Did this work?
17 strcpy(b, a);
18 sprintf(s, "\n");
19 sprintf(s, "hello");
20 sprintf(s, "hello %s", bug);
21 sprintf(s, gettext("hello %s"), bug);
22 sprintf(s, unknown, bug);
23 printf(bf, x);
24 scanf("%d", &x);
25 scanf("%s", s);
26 scanf("%10s", s);
27 scanf("%s", s);
28 gets(f); // Flawfinder: ignore
29 printf("\\");
30 /* Flawfinder: ignore */
31 gets(f);
32 gets(f);
33 /* These are okay, but flawfinder version < 0.20 incorrectly used
34 the first parameter as the parameter for the format string */
35 syslog(LOG_ERR,"cannot open config file (%s): %s",filename,strerror(errno))
36 syslog(LOG_CRIT,"malloc() failed");
37 /* But this one SHOULD trigger a warning. */
38 syslog(LOG_ERR, attacker_string);
39
40 }
41
42
43
44 demo2() {
45 char d[20];
46 char s[20];
47 int n;
48
49 _mbscpy(d,s); /* like strcpy, this doesn't check for buffer overflow */
50 memcpy(d,s);
51 CopyMemory(d,s);
52 lstrcat(d,s);
53 strncpy(d,s);
54 _tcsncpy(d,s);
55 strncat(d,s,10);
56 strncat(d,s,sizeof(d)); /* Misuse - this should be flagged as riskier. */
57 _tcsncat(d,s,sizeof(d)); /* Misuse - flag as riskier */
58 n = strlen(d);
59 /* This is wrong, and should be flagged as risky: */
60 MultiByteToWideChar(CP_ACP,0,szName,-1,wszUserName,sizeof(wszUserName));
61 /* This is also wrong, and should be flagged as risky: */
62 MultiByteToWideChar(CP_ACP,0,szName,-1,wszUserName,sizeof wszUserName);
63 /* This is much better: */
64 MultiByteToWideChar(CP_ACP,0,szName,-1,wszUserName,sizeof(wszUserName)/sizeof(wszUserName[0]));
65 /* This is much better: */
66 MultiByteToWideChar(CP_ACP,0,szName,-1,wszUserName,sizeof wszUserName /sizeof(wszUserName[0]));
67 /* This is an example of bad code - the third paramer is NULL, so it creates
68 a NULL ACL. Note that Flawfinder can't detect when a
69 SECURITY_DESCRIPTOR structure is manually created with a NULL value
70 as the ACL; doing so would require a tool that handles C/C++
71 and knows about types more that flawfinder currently does.
72 Anyway, this needs to be detected: */
73 SetSecurityDescriptorDacl(&sd,TRUE,NULL,FALSE);
74 /* This one is a bad idea - first param shouldn't be NULL */
75 CreateProcess(NULL, "C:\\Program Files\\GoodGuy\\GoodGuy.exe -x", "");
76 /* Test interaction of quote characters */
77 printf("%c\n", 'x');
78 printf("%c\n", '"');
79 printf("%c\n", '\"');
80 printf("%c\n", '\'');
81 printf("%c\n", '\177');
82 printf("%c\n", '\xfe');
83 printf("%c\n", '\xd');
84 printf("%c\n", '\n');
85 printf("%c\n", '\\');
86 printf("%c\n", "'");
87 }
88
89
90 int getopt_example(int argc,char *argv[]) {
91 while ((optc = getopt_long (argc, argv, "a",longopts, NULL )) != EOF) {
92 }
93 }
94
95 int testfile() {
96 FILE *f;
97 f = fopen("/etc/passwd", "r");
98 fclose(f);
99 }
100
101 /* Regression test: handle \\\n after end of string */
102
103 #define assert(x) {\
104 if (!(x)) {\
105 fprintf(stderr,"Assertion failed.\n"\
106 "File: %s\nLine: %d\n"\
107 "Assertion: %s\n\n"\
108 ,__FILE__,__LINE__,#x);\
109 exit(1);\
110 };\
111 }
112
113 int accesstest() {
114 int access = 0; /* Not a function call. Should be caught by the
115 false positive test, and NOT labelled as a problem. */
116 }
実行結果
andre@andre-VirtualBox:~/work/flawfinder$ cat flawfinder_out.txt
Flawfinder version 1.27, (C) 2001-2004 David A. Wheeler.
Number of dangerous functions in C/C++ ruleset: 160
Examining test.c
test.c:32: [5] (buffer) gets:
Does not check for buffer overflows. Use fgets() instead.
★↑バッファオーバフローのためのチェックしていない。gets()->fgets()へ置き換えるべき。
test.c:56: [5] (buffer) strncat:
Easily used incorrectly (e.g., incorrectly computing the correct
maximum size to add). Consider strlcat or automatically resizing strings.
★↑誤って使用されやすい。
★↑strncat()ではなく、strlcat()もしくは、自動的にリサイズする文字列型？を考えるべき。
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). Consider 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. Risk is high, it
appears that the size is given as bytes, but the function requires size as
characters.
★↑バイト数ではなく、文字の最大長が引数に要求されている。高リスクであり、バイト数を与えるべき。
test.c:62: [5] (buffer) MultiByteToWideChar:
Requires maximum length in CHARACTERS, not bytes. Risk is high, it
appears that the size is given as bytes, but the function requires size as
characters.
test.c:73: [5] (misc) SetSecurityDescriptorDacl:
Never create NULL ACLs; an attacker can set it to Everyone (Deny All
Access), which would even forbid administrator access.
test.c:73: [5] (misc) SetSecurityDescriptorDacl:
Never create NULL ACLs; an attacker can set it to Everyone (Deny All
Access), which would even forbid administrator access.
test.c:17: [4] (buffer) strcpy:
Does not check for buffer overflows when copying to destination.
★↑連結元へのコピー時にバッファオーバフローが考慮されていない。
Consider using strncpy or strlcpy (warning, strncpy is easily misused).
★strncpy か strlcpyへの代替を考慮すべき。( 警告：strncpyは誤使用しやすい）
test.c:20: [4] (buffer) sprintf:
Does not check for buffer overflows. Use snprintf or vsnprintf.
★バッファオーバフローが考慮されていない。snprintf か vsnprintfを使うべき。
test.c:21: [4] (buffer) sprintf:
Does not check for buffer overflows. Use snprintf or vsnprintf.
test.c:22: [4] (format) sprintf:
Potential format string problem. Make format string constant.
★書式部分が文字列ではない可能性あり。文字列定数であるべき。
test.c:23: [4] (format) printf:
If format strings can be influenced by an attacker, they can be
exploited. Use a constant for the format specification.
★書式部分は、クラッカーによって悪用されることがしばしばある。そのため、書式部分は文字定数を使うべき。
test.c:25: [4] (buffer) scanf:
The scanf() family's %s operation, without a limit specification,
permits buffer overflows. Specify a limit to %s, or use a different input
function.
★制限のない%s書式は、バッファオーバフローを許してしまう。上限制限(%5s等)をつけるか、ほかの関数を使用するべき。
test.c:27: [4] (buffer) scanf:
The scanf() family's %s operation, without a limit specification,
permits buffer overflows. Specify a limit to %s, or use a different input
function.
test.c:38: [4] (format) syslog:
If syslog's format strings can be influenced by an attacker, they can
be exploited. Use a constant format string for syslog.
test.c:49: [4] (buffer) _mbscpy:
Does not check for buffer overflows when copying to destination.
Consider using a function version that stops copying at the end of the
buffer.
★コピー先へのコピー時、バッファオーバフローが考慮されていない。
★バッファの終端でコピーを止める関数を使うことを考えるべき。
test.c:52: [4] (buffer) lstrcat:
Does not check for buffer overflows when concatenating to destination.
★結合先への文字列結合時、バッファオーバフローが考慮されていない。
test.c:75: [3] (shell) CreateProcess:
This causes a new process to execute and is difficult to use safely.
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:75: [3] (shell) CreateProcess:
This causes a new process to execute and is difficult to use safely.
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:91: [3] (buffer) getopt_long:
Some older implementations do not protect against internal buffer
overflows . Check implementation on installation, or limit the size of all
string inputs.
★関数内部でのバッファオーバフローに対して、保護されていない。
★インストール時に実装を確認するか、入力文字列すべてのサイズに制限をかけるべき。
test.c:16: [2] (buffer) strcpy:
Does not check for buffer overflows when copying to destination.
Consider using strncpy or strlcpy (warning, strncpy is easily misused). Risk
is low because the source is a constant string.
★コピー先へのコピー時、バッファオーバフローが考慮されていない。
★strncpy か strlcpy の使用を考えるべき（警告：strncpyは誤使用されやすい）
★コピー元が文字列定数であるため、低リスクである。
test.c:19: [2] (buffer) sprintf:
Does not check for buffer overflows. Use snprintf or vsnprintf. Risk
is low because the source has a constant maximum length.
★バッファオーバフローを考慮すべきである。snprintf か vsnprintfを使うべきである。
★文字列定数が指定されており、最大長が決まっているため、低リスクである。
test.c:45: [2] (buffer) char:
Statically-sized arrays can be overflowed. Perform bounds checking,
use functions that limit length, or ensure that the size is larger than
the maximum possible length.
★静的サイズの配列がオーバーフローすることができます。
★境界チェックを実行し、長さを制限する機能を使用するか、
★またはサイズが使用可能な最大長より大きいことを確認してください。( by google翻訳 )
test.c:46: [2] (buffer) char:
Statically-sized arrays can be overflowed. Perform bounds checking,
use functions that limit length, or ensure that the size is larger than
the maximum possible length.
test.c:50: [2] (buffer) memcpy:
Does not check for buffer overflows when copying to destination. Make
sure destination can always hold the source data.
★コピー先へのコピー時、バッファオーバフローを考慮すべき。
★コピー元のデータが、コピー先へ格納できるかいつも確認することが必要である。
test.c:51: [2] (buffer) CopyMemory:
Does not check for buffer overflows when copying to destination. Make
sure destination can always hold the source data.
test.c:97: [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?.
test.c:15: [1] (buffer) strcpy:
Does not check for buffer overflows when copying to destination.
Consider using strncpy or strlcpy (warning, strncpy is easily misused). Risk
is low because the source is a constant character.
test.c:18: [1] (buffer) sprintf:
Does not check for buffer overflows. Use snprintf or vsnprintf. Risk
is low because the source is a constant character.
test.c:26: [1] (buffer) scanf:
it's unclear if the %s limit in the format string is small enough.
Check that the limit is sufficiently small, or use a different input
function.
test.c:53: [1] (buffer) strncpy:
Easily used incorrectly; doesn't always \0-terminate or check for
invalid pointers.
test.c:54: [1] (buffer) _tcsncpy:
Easily used incorrectly; doesn't always \0-terminate or check for
invalid pointers.
test.c:55: [1] (buffer) strncat:
Easily used incorrectly (e.g., incorrectly computing the correct
maximum size to add). Consider strlcat or automatically resizing strings.
test.c:58: [1] (buffer) strlen:
Does not handle strings that are not \0-terminated (it could cause a
crash if unprotected).
★NULL終端がない文字列かもしれない。クラッシュするかも…
test.c:64: [1] (buffer) MultiByteToWideChar:
Requires maximum length in CHARACTERS, not bytes. Risk is very low,
the length appears to be in characters not bytes.
test.c:66: [1] (buffer) MultiByteToWideChar:
Requires maximum length in CHARACTERS, not bytes. Risk is very low,
the length appears to be in characters not bytes.
Hits = 36
Lines analyzed = 116 in 0.54 seconds (3259 lines/second)
Physical Source Lines of Code (SLOC) = 80
Hits@level = [0] 0 [1] 9 [2] 7 [3] 3 [4] 10 [5] 7
Hits@level+ = [0+] 36 [1+] 36 [2+] 27 [3+] 20 [4+] 17 [5+] 7
Hits/KSLOC@level+ = [0+] 450 [1+] 450 [2+] 337.5 [3+] 250 [4+] 212.5 [5+] 87.5
Suppressed hits = 2 (use --neverignore to show them)
Minimum risk level = 1
Not every hit is necessarily a security vulnerability.
There may be other security vulnerabilities; review your code!
（※ 訳しづらいものorできなかったものは、訳していない）
考察
独自の関数(lstrcat等)についても警告を出すようだ。関数の文字列から、そこらへんは判断しているみたい。
strncpyやstrncatではなく、もっと安全なstrlcpyやstrlcatがあるとは知らなかった。
