I had the pleasure of looking at some of the FNs on the Juliet benchmark for a week.
I focused on the following CWEs in this order:
- CWE-787: Out-of-bounds Write CWE-123, CWE-124, CWE-121, CWE-122
- CWE-78: OS Command Injection CWE-78
- CWE-125: Out-of-bounds Read CWE-126
- CWE-22: Path Traversal CWE-23, CWE-36
While looking at the cases, I noticed some repeating patterns and a few easy fixes, along with some hacky and debatable workarounds for catching those FNs.
Given that I only spent a single week on this evaluation, I don’t want to share the results just yet, and the title reflects this. Rather, my intention is to raise awareness that I’d like to upstream some of the quick fixes I applied during that week so that everyone could benefit from this.
I have in total 13 commits, related to:
- CStringChecker (2)
- ArrayBoundsV2 (3)
- GenericTaintChecker (8)
I’d like to upstream them in two batches so that I don’t need to split them from a branch and test them one by one if it’s okay.
The first batch would be for the CStringChecker and ArrayBoundsV2 together, and then the second batch would be for the taint improvements.
Here is the first batch:
D159105 ArrayBoundCheckerV2 should check the region for taint as well
Causes FPs when code uses the result of
getenv, e.g. manually parses the content.
D159106 ArrayBoundCheckerV2 should listen to check::Bind as well
Would introduce FPs for zero-sized non-trivial object arrays, in which case we the engine
VisitDeclStmthandler. This issue seems to be orthogonal, but this patch would depend on fixing that first.
D159107 ArrayBoundCheckerV2 should disallow forming lvalues to out-of-bounds locations
Without looking around in the AST, we cannot tell if the lvalue (e.g. one-past-last element, as a sentinel) is never read from (not a child of an
LValueToRValuecast). Diagnosing and halting the abstract machine in such cases does more harm than good.
- (Landed) D159108 CStringChecker should check the first byte of the destination of strcpy, strncpy
- (Landed) D159109 CStringChecker buffer access checks should check the first bytes
Here is the first batch for improving taint analysis:
(Under review) Fix taint sink rules for exec-like functions
Rest of the patches (TBD).
To set expectations, by landing these 13 commits, we won’t improve the FN rate a lot, it would need a little bit more engineering to cover a significant portion of the FNs. Stay tuned for a follow-up post about the details of what needs to be done to improve on this - regarding the Juliet benchmark. No ETA.