dkrupp wrote:
If we remove the malloc(..) as the taint sink, we would lose some true positive
findings where the size of the allocated
area is specified directly as a number by the attacker:
```
char *size=getenv("SIZE");
if (size){
pathbuf=(char*) malloc(atoi(size)+1); // warn: denial of service attack!
...
}
```
The above example is prone to denial of service attack as the the attacker just
specifies an arbitrarily large number to which a buffer will be allocated. The
attacker needs much less resources to specify a large number than the recevier
to allocate a large chuck of memory.
On the other hand when we have a code like this:
```
char *user_txt=getenv("SIZE");
if (user_txt){
pathbuf=(char*) malloc(strlen(user_txt)+1); // No Warning as the malloc
parameter comes from the size of an already allocated buffer
...
}
```
Here we should not warn as the size passed to malloc is the size of an already
allocated buffer. So invested resources by the attacker to provide the large
string and the server allocating another buffer to contain that string is
symmetrical. So not prone to DoS attack.
A more sophisticated longer term solution could be that we add a flag to the
taint info (or introduce a taint type) that the tainted value was originating
from an existing buffer size and then specify the malloc sink so that it should
not warn in that case. I know we cannot do this know, but the taint analysis
could be extended into this direction.
Back to this solution.
Please note that this is only the default configuration of the checker.
The user could add the stren as a propagator into the taint config file.
If we decide to remove strlen() as a propagator (as is in this patch) we could
highlight this in the documentation of the checker that the user may want to
add it back.
So for me either solution would work:
a) remove strlen() as a propagator and note it in the checker doc
b) remove malloc() as a sink and note it in the checker doc
c) don't do anything and live with the false positives
Which one would you prefer?
https://github.com/llvm/llvm-project/pull/66086
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
