llvm / llvm/llvm-project

[analyzer] `security.ArrayBound` false positive

Open
#167,887 4 comments 0 reactions 0 assignees View on GitHub
clang:static analyzer false-positive
Dominant language
LLVM
Stars
40.5k
Forks
18.7k
PR merge metrics
PR metrics pending

Description

```c
#include
#include
#include
#include

#include

void foo(size_t max_path_len)
{
assert(max_path_len < SIZE_MAX);
char *buf = malloc(max_path_len + 1);
assert(buf);

int path_len_signed = readlink("hello", buf, max_path_len);
assert(path_len_signed >= 0);
size_t path_len = (size_t)path_len_signed;
assert(path_len <= max_path_len);

buf[path_len] = 0;
}
```

Here the access to `buf[path_len]` is always valid, but the analyzer doesn't think so. This happens even if I change `SIZE_MAX` in the first assert to `5`, so it's not anything to do with huge allocations.

```ShellSession
$ clang-tidy repro.c
Error while trying to load a compilation database:
Could not auto-detect compilation database for file "repro.c"
No compilation database found in /tmp or any parent directory
fixed-compilation-database: Error while opening fixed database: No such file or directory
json-compilation-database: Error while opening JSON database: No such file or directory
Running without flags.
3 warnings generated.
/nix/store/cz0l7aa2dzk2sgz9b55i1wzn0v80ygwq-glibc-2.40-66-dev/include/stdint.h:95:11: warning: '__INT64_C' macro redefined [clang-diagnostic-macro-redefined]
95 | # define __INT64_C(c) c ## L
| ^
note: previous definition is here
/nix/store/cz0l7aa2dzk2sgz9b55i1wzn0v80ygwq-glibc-2.40-66-dev/include/stdint.h:96:11: warning: '__UINT64_C' macro redefined [clang-diagnostic-macro-redefined]
96 | # define __UINT64_C(c) c ## UL
| ^
note: previous definition is here
/tmp/repro.c:19:2: warning: Potential out of bound access to the heap area with tainted index [clang-analyzer-security.ArrayBound]
19 | buf[path_len] = 0;
| ^~~~~~~~~~~~~
/tmp/repro.c:10:9: note: Assuming 'max_path_len' is < -1
10 | assert(max_path_len < SIZE_MAX);
| ^
/nix/store/cz0l7aa2dzk2sgz9b55i1wzn0v80ygwq-glibc-2.40-66-dev/include/assert.h:117:11: note: expanded from macro 'assert'
117 | if (expr) \
| ^~~~
/tmp/repro.c:10:2: note: Taking true branch
10 | assert(max_path_len < SIZE_MAX);
| ^
/nix/store/cz0l7aa2dzk2sgz9b55i1wzn0v80ygwq-glibc-2.40-66-dev/include/assert.h:117:7: note: expanded from macro 'assert'
117 | if (expr) \
| ^
/tmp/repro.c:12:9: note: Assuming 'buf' is non-null
12 | assert(buf);
| ^
/tmp/repro.c:12:2: note: Taking true branch
12 | assert(buf);
| ^
/nix/store/cz0l7aa2dzk2sgz9b55i1wzn0v80ygwq-glibc-2.40-66-dev/include/assert.h:117:7: note: expanded from macro 'assert'
117 | if (expr) \
| ^
/tmp/repro.c:14:24: note: Taint originated here
14 | int path_len_signed = readlink("hello", buf, max_path_len);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/repro.c:14:24: note: Assuming that the 3rd argument to 'readlink' is <= 18446744073709551615
14 | int path_len_signed = readlink("hello", buf, max_path_len);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/repro.c:14:24: note: Taint propagated to the return value
14 | int path_len_signed = readlink("hello", buf, max_path_len);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/repro.c:14:24: note: Assuming that 'readlink' is successful
14 | int path_len_signed = readlink("hello", buf, max_path_len);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/repro.c:15:9: note: 'path_len_signed' is >= 0
15 | assert(path_len_signed >= 0);
| ^
/tmp/repro.c:15:2: note: Taking true branch
15 | assert(path_len_signed >= 0);
| ^
/nix/store/cz0l7aa2dzk2sgz9b55i1wzn0v80ygwq-glibc-2.40-66-dev/include/assert.h:117:7: note: expanded from macro 'assert'
117 | if (expr) \
| ^
/tmp/repro.c:17:9: note: 'path_len' is <= 'max_path_len'
17 | assert(path_len <= max_path_len);
| ^
/tmp/repro.c:17:2: note: Taking true branch
17 | assert(path_len <= max_path_len);
| ^
/nix/store/cz0l7aa2dzk2sgz9b55i1wzn0v80ygwq-glibc-2.40-66-dev/include/assert.h:117:7: note: expanded from macro 'assert'
117 | if (expr) \
| ^
/tmp/repro.c:19:2: note: Access of the heap area with a tainted index that may be too large
19 | buf[path_len] = 0;
| ^~~~~~~~~~~~~
```

Contributor guide

Open the contributing guide

Research direction

Start by reproducing the warning with repro.c and the clang-tidy security.ArrayBound checker using the command shown. Trace how the checker reasons about the readlink result, the assertions, and the heap index. Done means this valid buf[path_len] access no longer produces the false-positive warning while genuine out-of-bounds accesses remain reported.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
devtools, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.