Clang buffer overflow checks fail to detect simple case.

[llvmbot]

Bugzilla Link	11114
Version	trunk
OS	Linux
Reporter	LLVM Bugzilla Contributor
CC	@AnnaZaks,@tkremenek

Extended Description

Tested with version of clang/scan-build in trunk. The static analyzer fails to detect a simple buffer overflow in program found here. I guess more of an FYI than anything else..

http://www.debian-administration.org/articles/408

clang -v
clang version 3.0 (trunk 141707)
Target: x86_64-unknown-linux-gnu
Thread model: posix

scan-build gcc -o buggy buggy.c
scan-build: 'clang' executable not found in '/opt/clang/scan-build/bin'.
scan-build: Using 'clang' from path: /opt/clang/bin/clang
scan-build: Removing directory '/tmp/scan-build-2011-10-11-1' because it contains no reports.

clang --analyze -Xclang -analyzer-checker -Xclang security.experimental buggy.c
clang --analyze -Xclang -analyzer-checker -Xclang security.experimental.ArrayBound buggy.c
clang --analyze -Xclang -analyzer-checker -Xclang security.experimental.ArrayBound2 buggy.c

[llvmbot]

assigned to @tkremenek

[llvmbot]

Analyzer fails to warn for this even more obvious string overflow

[llvmbot]

I successfully reproduced this behavior myself on a recent version of the trunk (r145146). Since the example code you linked to is very blatantly vulnerable to a buffer overflow, I too would expect the analyzer to throw up a warning. Sure enough, it fails to do so for any of the invocations you gave.

Just to be safe, I double-checked the list of registered checkers with:

clang -cc1 -analyzer-checker-help

to see if there might be some other relevant checker that ought to be applied, but none stand out.

One guess is that, although this is an issue with array bounds, the checker fails this case because there's nothing in the buggy program to indicate what the bound on argv[1] might be. However, this doesn't appear to be the case either, as it seems that the static analyzer fails to catch other bad uses of strcpy() where information on the all the string lengths is explicit in the source text. It would be tempting to blame strcpy() itself, and to hypothesize that the checks fail because the overflowing operations occur in an externally linked chunk of code that is not being checked, i.e. <string.h>. However, the checks also fail for a naive strpcy() implemented by hand. Attached is a very small test case showing a very blatant string overflow that is not caught by the analyzer.

It appears that the analyzer simply fails to catch the bad string copy, even when the indices are made explicit and nothing is hidden behind function parameters.

How mature are the security checks at this stage? I know that they're still marked 'experimental', and so it might be that the necessary functionality simply hasn't been written into them yet. Is this a case of features that haven't been fully implemented, or is this a bug in mostly finished code?

[tkremenek]

How mature are the security checks at this stage? I know that they're still
marked 'experimental', and so it might be that the necessary functionality
simply hasn't been written into them yet. Is this a case of features that
haven't been fully implemented, or is this a bug in mostly finished code?

They aren't mature at all, and are far from complete. They are, however, being actively worked on, so bug reports like these, with extensive analyses, are extremely useful.

[tkremenek]

clone to rdar://problem/10488474

[tkremenek]

How mature are the security checks at this stage? I know that they're still
marked 'experimental', and so it might be that the necessary functionality
simply hasn't been written into them yet.

Generally speaking, 'experimental' checkers have enough functionality in place for those actively working on improving the analyzer to use them. They may be raft with false positives, or may have many missing features.

[llvmbot]

I noticed that somewhere between the version I originally tested and the latest (r145757), changes have been made to 'ArrayBoundV2' that now correctly throw a warning for the previously attached example.

However, the analyzer again fails to throw a warning if the attachment code is modified so that the declaration:

char foo[1];

is replaced with:

char* foo = (char*) malloc(sizeof(char));

which, for these purposes, should exhibit the same behavior.