From 6589fc52756d031750a97ddffa2fbd0417fa5947 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= Date: Fri, 5 Jul 2024 11:27:09 +0200 Subject: [PATCH] ArithmeticUncontrolled: float exprs are barriers. This leads to two regressions, but this should be OK if we decide to drop float support from SimpleRangeAnalysis Keeping high query precision to keep it in code scanning. --- .../CWE/CWE-190/ArithmeticUncontrolled.ql | 4 ++- .../ArithmeticUncontrolled.expected | 20 -------------- .../semmle/ArithmeticUncontrolled/test.c | 26 +++++++++---------- .../semmle/ArithmeticUncontrolled/test.cpp | 6 ++--- 4 files changed, 19 insertions(+), 37 deletions(-) diff --git a/cpp/ql/src/Security/CWE/CWE-190/ArithmeticUncontrolled.ql b/cpp/ql/src/Security/CWE/CWE-190/ArithmeticUncontrolled.ql index b660c53e2439..90e0ae2fc69c 100644 --- a/cpp/ql/src/Security/CWE/CWE-190/ArithmeticUncontrolled.ql +++ b/cpp/ql/src/Security/CWE/CWE-190/ArithmeticUncontrolled.ql @@ -5,7 +5,7 @@ * @kind path-problem * @problem.severity warning * @security-severity 8.6 - * @precision medium + * @precision high * @id cpp/uncontrolled-arithmetic * @tags security * external/cwe/cwe-190 @@ -117,6 +117,8 @@ module UncontrolledArithConfig implements DataFlow::ConfigSig { op instanceof ComplementExpr ).getAnOperand*() or + node.asExpr().getUnderlyingType() instanceof FloatingPointType + or // block unintended flow to pointers node.asExpr().getUnspecifiedType() instanceof PointerType } diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/ArithmeticUncontrolled.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/ArithmeticUncontrolled.expected index ec0fc949d60a..994d6fd7bf2a 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/ArithmeticUncontrolled.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/ArithmeticUncontrolled.expected @@ -11,11 +11,6 @@ edges | test.c:81:13:81:29 | ... ^ ... | test.c:83:9:83:9 | r | provenance | | | test.c:81:14:81:17 | call to rand | test.c:81:13:81:29 | ... ^ ... | provenance | | | test.c:81:23:81:26 | call to rand | test.c:81:13:81:29 | ... ^ ... | provenance | | -| test.c:104:13:104:16 | call to rand | test.c:104:13:104:16 | call to rand | provenance | | -| test.c:104:13:104:16 | call to rand | test.c:105:5:105:42 | ... = ... | provenance | | -| test.c:105:5:105:42 | ... = ... | test.c:106:13:106:13 | r | provenance | | -| test.c:106:13:106:13 | r | test.c:110:18:110:18 | r | provenance | | -| test.c:110:18:110:18 | r | test.c:111:3:111:3 | r | provenance | | | test.c:125:13:125:16 | call to rand | test.c:125:13:125:16 | call to rand | provenance | | | test.c:125:13:125:16 | call to rand | test.c:127:9:127:9 | r | provenance | | | test.c:131:13:131:16 | call to rand | test.c:131:13:131:16 | call to rand | provenance | | @@ -47,9 +42,6 @@ edges | test.cpp:151:10:151:13 | call to rand | test.cpp:151:10:151:13 | call to rand | provenance | | | test.cpp:151:10:151:13 | call to rand | test.cpp:153:10:153:15 | ... - ... | provenance | | | test.cpp:153:10:153:15 | ... - ... | test.cpp:154:10:154:10 | b | provenance | | -| test.cpp:169:11:169:14 | call to rand | test.cpp:169:11:169:14 | call to rand | provenance | | -| test.cpp:169:11:169:14 | call to rand | test.cpp:170:13:170:13 | x | provenance | | -| test.cpp:170:13:170:13 | x | test.cpp:171:11:171:16 | y | provenance | | | test.cpp:189:10:189:13 | call to rand | test.cpp:189:10:189:13 | call to rand | provenance | | | test.cpp:189:10:189:13 | call to rand | test.cpp:195:3:195:11 | ... = ... | provenance | | | test.cpp:189:10:189:13 | call to rand | test.cpp:198:3:198:11 | ... = ... | provenance | | @@ -82,12 +74,6 @@ nodes | test.c:81:14:81:17 | call to rand | semmle.label | call to rand | | test.c:81:23:81:26 | call to rand | semmle.label | call to rand | | test.c:83:9:83:9 | r | semmle.label | r | -| test.c:104:13:104:16 | call to rand | semmle.label | call to rand | -| test.c:104:13:104:16 | call to rand | semmle.label | call to rand | -| test.c:105:5:105:42 | ... = ... | semmle.label | ... = ... | -| test.c:106:13:106:13 | r | semmle.label | r | -| test.c:110:18:110:18 | r | semmle.label | r | -| test.c:111:3:111:3 | r | semmle.label | r | | test.c:125:13:125:16 | call to rand | semmle.label | call to rand | | test.c:125:13:125:16 | call to rand | semmle.label | call to rand | | test.c:127:9:127:9 | r | semmle.label | r | @@ -130,10 +116,6 @@ nodes | test.cpp:151:10:151:13 | call to rand | semmle.label | call to rand | | test.cpp:153:10:153:15 | ... - ... | semmle.label | ... - ... | | test.cpp:154:10:154:10 | b | semmle.label | b | -| test.cpp:169:11:169:14 | call to rand | semmle.label | call to rand | -| test.cpp:169:11:169:14 | call to rand | semmle.label | call to rand | -| test.cpp:170:13:170:13 | x | semmle.label | x | -| test.cpp:171:11:171:16 | y | semmle.label | y | | test.cpp:189:10:189:13 | call to rand | semmle.label | call to rand | | test.cpp:189:10:189:13 | call to rand | semmle.label | call to rand | | test.cpp:190:10:190:13 | call to rand | semmle.label | call to rand | @@ -160,7 +142,6 @@ subpaths | test.c:77:9:77:9 | r | test.c:75:13:75:19 | call to rand | test.c:77:9:77:9 | r | This arithmetic expression depends on an $@, potentially causing an overflow. | test.c:75:13:75:19 | call to rand | uncontrolled value | | test.c:83:9:83:9 | r | test.c:81:14:81:17 | call to rand | test.c:83:9:83:9 | r | This arithmetic expression depends on an $@, potentially causing an overflow. | test.c:81:14:81:17 | call to rand | uncontrolled value | | test.c:83:9:83:9 | r | test.c:81:23:81:26 | call to rand | test.c:83:9:83:9 | r | This arithmetic expression depends on an $@, potentially causing an overflow. | test.c:81:23:81:26 | call to rand | uncontrolled value | -| test.c:111:3:111:3 | r | test.c:104:13:104:16 | call to rand | test.c:111:3:111:3 | r | This arithmetic expression depends on an $@, potentially causing an overflow. | test.c:104:13:104:16 | call to rand | uncontrolled value | | test.c:127:9:127:9 | r | test.c:125:13:125:16 | call to rand | test.c:127:9:127:9 | r | This arithmetic expression depends on an $@, potentially causing an overflow. | test.c:125:13:125:16 | call to rand | uncontrolled value | | test.c:133:5:133:5 | r | test.c:131:13:131:16 | call to rand | test.c:133:5:133:5 | r | This arithmetic expression depends on an $@, potentially causing an overflow. | test.c:131:13:131:16 | call to rand | uncontrolled value | | test.c:139:10:139:10 | r | test.c:137:13:137:16 | call to rand | test.c:139:10:139:10 | r | This arithmetic expression depends on an $@, potentially causing an overflow. | test.c:137:13:137:16 | call to rand | uncontrolled value | @@ -172,7 +153,6 @@ subpaths | test.cpp:102:10:102:10 | x | test.cpp:98:10:98:13 | call to rand | test.cpp:102:10:102:10 | x | This arithmetic expression depends on an $@, potentially causing an overflow. | test.cpp:98:10:98:13 | call to rand | uncontrolled value | | test.cpp:146:9:146:9 | y | test.cpp:137:10:137:13 | call to rand | test.cpp:146:9:146:9 | y | This arithmetic expression depends on an $@, potentially causing an overflow. | test.cpp:137:10:137:13 | call to rand | uncontrolled value | | test.cpp:154:10:154:10 | b | test.cpp:151:10:151:13 | call to rand | test.cpp:154:10:154:10 | b | This arithmetic expression depends on an $@, potentially causing an overflow. | test.cpp:151:10:151:13 | call to rand | uncontrolled value | -| test.cpp:171:11:171:16 | y | test.cpp:169:11:169:14 | call to rand | test.cpp:171:11:171:16 | y | This arithmetic expression depends on an $@, potentially causing an overflow. | test.cpp:169:11:169:14 | call to rand | uncontrolled value | | test.cpp:196:7:196:7 | x | test.cpp:189:10:189:13 | call to rand | test.cpp:196:7:196:7 | x | This arithmetic expression depends on an $@, potentially causing an overflow. | test.cpp:189:10:189:13 | call to rand | uncontrolled value | | test.cpp:198:7:198:7 | x | test.cpp:189:10:189:13 | call to rand | test.cpp:198:7:198:7 | x | This arithmetic expression depends on an $@, potentially causing an overflow. | test.cpp:189:10:189:13 | call to rand | uncontrolled value | | test.cpp:199:7:199:7 | x | test.cpp:189:10:189:13 | call to rand | test.cpp:199:7:199:7 | x | This arithmetic expression depends on an $@, potentially causing an overflow. | test.cpp:189:10:189:13 | call to rand | uncontrolled value | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/test.c b/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/test.c index d3fab6bb4b2c..0d5399b7b94c 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/test.c +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/test.c @@ -16,8 +16,8 @@ void randomTester() { int i; for (i = 0; i < 1000; i++) { int r = rand(); - - // BAD: The return from rand() is unbounded + + // BAD: The return from rand() is unbounded trySlice(r, r+100); } @@ -44,24 +44,24 @@ void randomTester() { int r = rand(); r += 100; // BAD } - + { int r = rand() / 10; r += 100; // GOOD } - + { int r = rand(); r = r / 10; r += 100; // GOOD } - + { int r = rand(); r /= 10; r += 100; // GOOD } - + { int r = rand() & 0xFF; r += 100; // GOOD @@ -108,7 +108,7 @@ void randomTester() { } void add_100(int r) { - r += 100; // GOOD [FALSE POSITIVE] + r += 100; // GOOD } void randomTester2(int bound, int min, int max) { @@ -123,13 +123,13 @@ void randomTester2(int bound, int min, int max) { void moreTests() { { int r = rand(); - + r = r * 100; // BAD } { int r = rand(); - + r *= 100; // BAD } @@ -141,19 +141,19 @@ void moreTests() { { int r = rand(); - + r <<= 8; // BAD [NOT DETECTED] } { int r = rand(); - + r = r - 100; // GOOD } { unsigned int r = rand(); - + r = r - 100; // BAD } } @@ -164,4 +164,4 @@ void guarded_test(unsigned p) { return; } unsigned z = data - p; // GOOD -} \ No newline at end of file +} diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/test.cpp b/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/test.cpp index 6df97ef54520..9115fb69e9eb 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/test.cpp +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/test.cpp @@ -128,7 +128,7 @@ int test_conditional_assignment_2() { y = x; } - + return y * 10; // GOOD (as y <= 100) } @@ -142,7 +142,7 @@ int test_conditional_assignment_3() { y = x; } - + return y * c; // GOOD (as y <= 100) [FALSE POSITIVE] } @@ -168,7 +168,7 @@ void test_float() { int x = rand(); float y = x; // GOOD - int z = (int)y * 5; // BAD + int z = (int)y * 5; // BAD [NOT DETECTED] } {