Skip to content

Commit a8c7dc3

Browse files
author
Nikita Kraiouchkine
committed
Restrict ARR30-C to reduce FPs and fix performance
1 parent b3523be commit a8c7dc3

File tree

4 files changed

+18
-6
lines changed

4 files changed

+18
-6
lines changed

c/cert/src/rules/ARR30-C/DoNotFormOutOfBoundsPointersOrArraySubscripts.ql

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,15 @@
2121
string message
2222
where
2323
not isExcluded(ba, OutOfBoundsPackage::doNotFormOutOfBoundsPointersOrArraySubscriptsQuery()) and
24+
// exclude loops
25+
not exists(Loop loop | loop.getStmt().getChildStmt*() = ba.getEnclosingStmt()) and
26+
// exclude size arguments that are of type ssize_t
27+
not sizeArg.getAChild*().(VariableAccess).getTarget().getType() instanceof Ssize_t and
28+
// exclude size arguments that are assigned the result of a function call e.g. ftell
29+
not sizeArg.getAChild*().(VariableAccess).getTarget().getAnAssignedValue() instanceof FunctionCall and
30+
// exclude field or array accesses for the size arguments
31+
not sizeArg.getAChild*() instanceof FieldAccess and
32+
not sizeArg.getAChild*() instanceof ArrayExpr and
2433
(
2534
exists(int sizeArgValue, int bufferArgSize |
2635
OOB::isSizeArgGreaterThanBufferSize(bufferArg, sizeArg, bufferSource, bufferArgSize, sizeArgValue, ba) and
@@ -33,7 +42,7 @@
3342
OOB::isSizeArgNotCheckedLessThanFixedBufferSize(bufferArg, sizeArg, bufferSource,
3443
bufferArgSize, ba, sizeArgUpperBound, sizeMult) and
3544
message =
36-
"Buffer accesses may access up to offset " + sizeArgUpperBound + "*" + sizeMult +
45+
"Buffer may access up to offset " + sizeArgUpperBound + "*" + sizeMult +
3746
" which is greater than the fixed size " + bufferArgSize + " of the $@."
3847
)
3948
or

c/cert/test/rules/ARR30-C/DoNotFormOutOfBoundsPointersOrArraySubscripts.expected

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
| test.c:16:3:16:13 | ... + ... | Buffer access may be to a negative index in the buffer. | test.c:16:3:16:5 | arr | buffer |
33
| test.c:21:5:21:15 | ... + ... | Buffer access may be to a negative index in the buffer. | test.c:21:5:21:7 | arr | buffer |
44
| test.c:41:17:41:30 | ... + ... | Buffer access may be to a negative index in the buffer. | test.c:41:17:41:22 | buffer | buffer |
5-
| test.c:45:17:45:30 | ... + ... | Buffer accesses may access up to offset 101*1 which is greater than the fixed size 100 of the $@. | test.c:45:17:45:22 | buffer | buffer |
5+
| test.c:45:17:45:30 | ... + ... | Buffer may access up to offset 101*1 which is greater than the fixed size 100 of the $@. | test.c:45:17:45:22 | buffer | buffer |
66
| test.c:55:5:55:13 | ... - ... | Buffer access may be to a negative index in the buffer. | test.c:55:5:55:9 | ptr16 | buffer |
77
| test.c:57:5:57:14 | ... + ... | Buffer accesses offset 22 which is greater than the fixed size 20 of the $@. | test.c:57:5:57:9 | ptr16 | buffer |
88
| test.c:58:5:58:14 | ... - ... | Buffer access may be to a negative index in the buffer. | test.c:58:5:58:9 | ptr16 | buffer |

c/common/src/codingstandards/c/OutOfBounds.qll

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -606,6 +606,7 @@ module OOB {
606606
SimpleStringLibraryFunctionCall() { this.getTarget() instanceof SimpleStringLibraryFunction }
607607
}
608608

609+
bindingset[dest]
609610
private Expr getSourceConstantExpr(Expr dest) {
610611
exists(result.getValue().toInt()) and
611612
DataFlow::localExprFlow(result, dest)
@@ -639,6 +640,7 @@ module OOB {
639640
* malloc(sz);
640641
* ```
641642
*/
643+
bindingset[e]
642644
private int getMinStatedValue(Expr e) {
643645
result = upperBound(e).minimum(min(getSourceConstantExpr(e).getValue().toInt()))
644646
}
@@ -647,6 +649,7 @@ module OOB {
647649
* A class for reasoning about the offset of a variable from the original value flowing to it
648650
* as a result of arithmetic or pointer arithmetic expressions.
649651
*/
652+
bindingset[expr]
650653
private int getArithmeticOffsetValue(Expr expr, Expr base) {
651654
result = getMinStatedValue(expr.(PointerArithmeticExpr).getOperand()) and
652655
base = expr.(PointerArithmeticExpr).getPointer()
@@ -1264,16 +1267,16 @@ module OOB {
12641267
) and
12651268
(
12661269
// Not a size expression for which we can compute a specific size
1267-
// and with a lower bound that is less than zero, taking into account offsets
12681270
not sizeExprComputableSize(sizeArg, _, _) and
1271+
// and with a lower bound that is less than zero, taking into account offsets
12691272
lowerBound(sizeArg) + getArithmeticOffsetValue(bufferArg, _) < 0
12701273
or
12711274
// A size expression for which we can compute a specific size and that size is less than zero
12721275
sizeExprComputableSize(sizeArg, _, _) and
12731276
(
12741277
if isSizeArgPointerSubExprRightOperand(sizeArg)
1275-
then -getMinStatedValue(sizeArg) + getArithmeticOffsetValue(bufferArg, _) < 0
1276-
else getMinStatedValue(sizeArg) + getArithmeticOffsetValue(bufferArg, _) < 0
1278+
then -sizeArg.getValue().toInt() + getArithmeticOffsetValue(bufferArg, _) < 0
1279+
else sizeArg.getValue().toInt() + getArithmeticOffsetValue(bufferArg, _) < 0
12771280
)
12781281
)
12791282
)

rule_packages/c/OutOfBounds.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
"description": "Forming or using an out-of-bounds pointer is undefined behavior and can result in invalid memory accesses.",
1010
"kind": "problem",
1111
"name": "Do not form or use out-of-bounds pointers or array subscripts",
12-
"precision": "high",
12+
"precision": "medium",
1313
"severity": "error",
1414
"short_name": "DoNotFormOutOfBoundsPointersOrArraySubscripts",
1515
"tags": [

0 commit comments

Comments
 (0)