Skip to content

C++: Block flow to integral and float types in cpp/unbounded-write #16144

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
MathiasVP wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

C++: Block flow to integral and float types in cpp/unbounded-write #16144

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
572600b
C++: Add testcase with FP.
MathiasVP Apr 8, 2024
9beaea5
C++: Block assignments to integral and float types.
MathiasVP Apr 8, 2024
90aa177
C++: Accept test changes.
MathiasVP Apr 8, 2024
920c987
C++: Add change note.
MathiasVP Apr 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions cpp/ql/src/Security/CWE/CWE-120/UnboundedWrite.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ module Config implements DataFlow::ConfigSig {
predicate isBarrier(DataFlow::Node node) {
// Block flow if the node is guarded by any <, <= or = operations.
node = DataFlow::BarrierGuard<lessThanOrEqual/3>::getABarrierNode()
or
// Block flow if we manage to flow to a single `int` or `float`.
node.asDefinition().getUnspecifiedType() instanceof IntegralType
or
node.asDefinition().getUnspecifiedType() instanceof FloatingPointType
}
}

Expand Down
4 changes: 4 additions & 0 deletions cpp/ql/src/change-notes/2024-04-08-unbounded-write.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The "Unbounded write" query (`cpp/unbounded-write`) has been improved to produce fewer false positives.
4 changes: 2 additions & 2 deletions cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/BadlyBoundedWrite.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
| tests.c:43:3:43:10 | call to snprintf | This 'call to snprintf' operation is limited to 111 bytes but the destination is only 110 bytes. |
| tests.c:46:3:46:10 | call to snprintf | This 'call to snprintf' operation is limited to 111 bytes but the destination is only 110 bytes. |
| tests.c:48:3:48:10 | call to snprintf | This 'call to snprintf' operation is limited to 111 bytes but the destination is only 110 bytes. |
| tests.c:51:3:51:10 | call to snprintf | This 'call to snprintf' operation is limited to 111 bytes but the destination is only 110 bytes. |
4 changes: 2 additions & 2 deletions cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/OverrunWriteFloat.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
| tests.c:103:3:103:9 | call to sprintf | This 'call to sprintf' operation may require 318 bytes because of float conversions, but the target is only 256 bytes. |
| tests.c:105:3:105:9 | call to sprintf | This 'call to sprintf' operation may require 346 bytes because of float conversions, but the target is only 256 bytes. |
| tests.c:108:3:108:9 | call to sprintf | This 'call to sprintf' operation may require 318 bytes because of float conversions, but the target is only 256 bytes. |
| tests.c:110:3:110:9 | call to sprintf | This 'call to sprintf' operation may require 346 bytes because of float conversions, but the target is only 256 bytes. |
22 changes: 11 additions & 11 deletions cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/VeryLikelyOverrunWrite.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,17 @@
| tests2.cpp:37:3:37:8 | call to wcscpy | This 'call to wcscpy' operation requires 28 bytes but the destination is only 24 bytes. |
| tests2.cpp:42:3:42:8 | call to wcscpy | This 'call to wcscpy' operation requires 32 bytes but the destination is only 28 bytes. |
| tests2.cpp:47:3:47:8 | call to wcscpy | This 'call to wcscpy' operation requires 36 bytes but the destination is only 32 bytes. |
| tests.c:54:3:54:9 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 10 bytes. |
| tests.c:58:3:58:9 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 10 bytes. |
| tests.c:62:17:62:24 | buffer10 | This 'scanf string argument' operation requires 11 bytes but the destination is only 10 bytes. |
| tests.c:63:17:63:24 | buffer10 | This 'scanf string argument' operation requires 12 bytes but the destination is only 10 bytes. |
| tests.c:86:3:86:8 | call to strcpy | This 'call to strcpy' operation requires 6 bytes but the destination is only 5 bytes. |
| tests.c:93:3:93:8 | call to strcpy | This 'call to strcpy' operation requires 6 bytes but the destination is only 5 bytes. |
| tests.c:120:3:120:9 | call to sprintf | This 'call to sprintf' operation requires 17 bytes but the destination is only 1 bytes. |
| tests.c:121:3:121:9 | call to sprintf | This 'call to sprintf' operation requires 17 bytes but the destination is only 16 bytes. |
| tests.c:136:2:136:8 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 10 bytes. |
| tests.c:186:3:186:9 | call to sprintf | This 'call to sprintf' operation requires 9 bytes but the destination is only 2 bytes. |
| tests.c:189:3:189:9 | call to sprintf | This 'call to sprintf' operation requires 3 bytes but the destination is only 2 bytes. |
| tests.c:59:3:59:9 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 10 bytes. |
| tests.c:63:3:63:9 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 10 bytes. |
| tests.c:67:17:67:24 | buffer10 | This 'scanf string argument' operation requires 11 bytes but the destination is only 10 bytes. |
| tests.c:68:17:68:24 | buffer10 | This 'scanf string argument' operation requires 12 bytes but the destination is only 10 bytes. |
| tests.c:91:3:91:8 | call to strcpy | This 'call to strcpy' operation requires 6 bytes but the destination is only 5 bytes. |
| tests.c:98:3:98:8 | call to strcpy | This 'call to strcpy' operation requires 6 bytes but the destination is only 5 bytes. |
| tests.c:125:3:125:9 | call to sprintf | This 'call to sprintf' operation requires 17 bytes but the destination is only 1 bytes. |
| tests.c:126:3:126:9 | call to sprintf | This 'call to sprintf' operation requires 17 bytes but the destination is only 16 bytes. |
| tests.c:141:2:141:8 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 10 bytes. |
| tests.c:191:3:191:9 | call to sprintf | This 'call to sprintf' operation requires 9 bytes but the destination is only 2 bytes. |
| tests.c:194:3:194:9 | call to sprintf | This 'call to sprintf' operation requires 3 bytes but the destination is only 2 bytes. |
| unions.c:26:2:26:7 | call to strcpy | This 'call to strcpy' operation requires 21 bytes but the destination is only 16 bytes. |
| unions.c:27:2:27:7 | call to strcpy | This 'call to strcpy' operation requires 21 bytes but the destination is only 16 bytes. |
| var_size_struct.cpp:22:3:22:8 | call to strcpy | This 'call to strcpy' operation requires 10 bytes but the destination is only 9 bytes. |
5 changes: 5 additions & 0 deletions cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/tests.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ int main(int argc, char *argv[])
scanf("%i", i); // GOOD: no problems with non-strings
scanf("%i %s", i, buffer100); // BAD: second format parameter may overflow
sscanf(argv[0], "%s", buffer100); // BAD: argv[0] could be more than 100 characters

char* argv1 = argv[1];
char buffer_with_tainted_char[4] = {0};
buffer_with_tainted_char[0] = *argv1;
sprintf(buffer100, buffer_with_tainted_char); // GOOD: Only a single character in buffer_with_tainted_char is tainted
}

// Test cases for BadlyBoundedWrite.ql
Expand Down