[PATCH] D125524: [BoundV2] ArrayBoundV2 checks if the extent is tainted
Balázs Benics via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri May 13 06:13:33 PDT 2022
steakhal added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:208
if (state_exceedsUpperBound && state_withinUpperBound) {
- SVal ByteOffset = rawOffset.getByteOffset();
- if (isTainted(state, ByteOffset)) {
+ if (isTainted(state, *upperboundToCheck)) {
reportOOB(checkerContext, state_exceedsUpperBound, OOB_Tainted,
----------------
martong wrote:
> Could you please explain why we change `rawOffset` to `*upperBoundToCheck`? And perhaps the same explanation could infiltrate into the checker's code itself as a comment to `upperbound`.
In the test attached you can see that the extent is tainted, not the offset.
Thus checking the offset for taint won't suffice.
The bug condition should depend on the calculation itself, which is basically what is done here.
================
Comment at: clang/test/Analysis/taint-diagnostic-visitor.c:46-48
+ int *p = (int *)malloc(x + conj); // Generic taint checker forbids tainted allocation.
+ // expected-warning at -1 {{Untrusted data is used to specify the buffer size}}
+ // expected-note at -2 {{Untrusted data is used to specify the buffer size}}
----------------
martong wrote:
> Could we get rid of the seemingly unrelated malloc taint report by using an array on the stack?
No, we need the extent to be tainted.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125524/new/
https://reviews.llvm.org/D125524
More information about the cfe-commits
mailing list