Skip to content

C++: handle cast arrays properly in off-by-one query #13117

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

Merged
MathiasVP merged 3 commits into from
Jun 28, 2023
Merged

C++: handle cast arrays properly in off-by-one query #13117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d18fb64
C++: handle cast arrays properly in off-by-one query
May 10, 2023
f2cbbab
Merge branch 'main' into rdmarsh2/cpp/cobo-handle-array-casts
MathiasVP Jun 27, 2023
51176bd
C++: Add Geoffrey's testcases.
MathiasVP Jun 27, 2023
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
10 changes: 5 additions & 5 deletions cpp/ql/src/experimental/Security/CWE/CWE-193/ConstantSizeArrayOffByOne.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,14 @@ predicate isInvalidPointerDerefSink2(DataFlow::Node sink, Instruction i, string

predicate arrayTypeCand(ArrayType arrayType) {
any(Variable v).getUnspecifiedType() = arrayType and
exists(arrayType.getArraySize())
exists(arrayType.getByteSize())
}

pragma[nomagic]
predicate arrayTypeHasSizes(ArrayType arr, int baseTypeSize, int arraySize) {
bindingset[baseTypeSize]
pragma[inline_late]
predicate arrayTypeHasSizes(ArrayType arr, int baseTypeSize, int size) {
arrayTypeCand(arr) and
arr.getBaseType().getSize() = baseTypeSize and
arr.getArraySize() = arraySize
arr.getByteSize() / baseTypeSize = size
}

bindingset[pai]
Expand Down
50 changes: 50 additions & 0 deletions ...imental/query-tests/Security/CWE/CWE-193/constant-size/ConstantSizeArrayOffByOne.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,55 +1,102 @@
edges
| test.cpp:34:10:34:12 | buf | test.cpp:34:5:34:24 | access to array |
| test.cpp:35:10:35:12 | buf | test.cpp:35:5:35:22 | access to array |
| test.cpp:36:10:36:12 | buf | test.cpp:36:5:36:24 | access to array |
| test.cpp:39:14:39:16 | buf | test.cpp:39:9:39:19 | access to array |
| test.cpp:43:14:43:16 | buf | test.cpp:43:9:43:19 | access to array |
| test.cpp:48:10:48:12 | buf | test.cpp:48:5:48:24 | access to array |
| test.cpp:49:10:49:12 | buf | test.cpp:49:5:49:22 | access to array |
| test.cpp:50:10:50:12 | buf | test.cpp:50:5:50:24 | access to array |
| test.cpp:53:14:53:16 | buf | test.cpp:53:9:53:19 | access to array |
| test.cpp:57:14:57:16 | buf | test.cpp:57:9:57:19 | access to array |
| test.cpp:61:14:61:16 | buf | test.cpp:61:9:61:19 | access to array |
| test.cpp:70:33:70:33 | p | test.cpp:71:5:71:17 | access to array |
| test.cpp:70:33:70:33 | p | test.cpp:72:5:72:15 | access to array |
| test.cpp:76:26:76:46 | & ... | test.cpp:66:32:66:32 | p |
| test.cpp:76:32:76:34 | buf | test.cpp:76:26:76:46 | & ... |
| test.cpp:77:26:77:44 | & ... | test.cpp:66:32:66:32 | p |
| test.cpp:77:32:77:34 | buf | test.cpp:77:26:77:44 | & ... |
| test.cpp:79:27:79:34 | buf | test.cpp:70:33:70:33 | p |
| test.cpp:79:32:79:34 | buf | test.cpp:79:27:79:34 | buf |
| test.cpp:85:34:85:36 | buf | test.cpp:87:5:87:31 | access to array |
| test.cpp:85:34:85:36 | buf | test.cpp:88:5:88:27 | access to array |
| test.cpp:96:13:96:15 | arr | test.cpp:96:13:96:18 | access to array |
| test.cpp:111:17:111:19 | arr | test.cpp:111:17:111:22 | access to array |
| test.cpp:111:17:111:19 | arr | test.cpp:115:35:115:40 | access to array |
| test.cpp:111:17:111:19 | arr | test.cpp:119:17:119:22 | access to array |
| test.cpp:115:35:115:37 | arr | test.cpp:111:17:111:22 | access to array |
| test.cpp:115:35:115:37 | arr | test.cpp:115:35:115:40 | access to array |
| test.cpp:115:35:115:37 | arr | test.cpp:119:17:119:22 | access to array |
| test.cpp:119:17:119:19 | arr | test.cpp:111:17:111:22 | access to array |
| test.cpp:119:17:119:19 | arr | test.cpp:115:35:115:40 | access to array |
| test.cpp:119:17:119:19 | arr | test.cpp:119:17:119:22 | access to array |
| test.cpp:128:9:128:11 | arr | test.cpp:128:9:128:14 | access to array |
| test.cpp:134:25:134:27 | arr | test.cpp:136:9:136:16 | ... += ... |
| test.cpp:136:9:136:16 | ... += ... | test.cpp:138:13:138:15 | arr |
| test.cpp:143:18:143:21 | asdf | test.cpp:134:25:134:27 | arr |
| test.cpp:143:18:143:21 | asdf | test.cpp:143:18:143:21 | asdf |
| test.cpp:148:23:148:28 | buffer | test.cpp:150:5:150:11 | access to array |
| test.cpp:148:23:148:28 | buffer | test.cpp:151:5:151:11 | access to array |
| test.cpp:159:25:159:29 | array | test.cpp:161:5:161:10 | access to array |
| test.cpp:159:25:159:29 | array | test.cpp:162:5:162:10 | access to array |
nodes
| test.cpp:34:5:34:24 | access to array | semmle.label | access to array |
| test.cpp:34:10:34:12 | buf | semmle.label | buf |
| test.cpp:35:5:35:22 | access to array | semmle.label | access to array |
| test.cpp:35:10:35:12 | buf | semmle.label | buf |
| test.cpp:36:5:36:24 | access to array | semmle.label | access to array |
| test.cpp:36:10:36:12 | buf | semmle.label | buf |
| test.cpp:39:9:39:19 | access to array | semmle.label | access to array |
| test.cpp:39:14:39:16 | buf | semmle.label | buf |
| test.cpp:43:9:43:19 | access to array | semmle.label | access to array |
| test.cpp:43:14:43:16 | buf | semmle.label | buf |
| test.cpp:48:5:48:24 | access to array | semmle.label | access to array |
| test.cpp:48:10:48:12 | buf | semmle.label | buf |
| test.cpp:49:5:49:22 | access to array | semmle.label | access to array |
| test.cpp:49:10:49:12 | buf | semmle.label | buf |
| test.cpp:50:5:50:24 | access to array | semmle.label | access to array |
| test.cpp:50:10:50:12 | buf | semmle.label | buf |
| test.cpp:53:9:53:19 | access to array | semmle.label | access to array |
| test.cpp:53:14:53:16 | buf | semmle.label | buf |
| test.cpp:57:9:57:19 | access to array | semmle.label | access to array |
| test.cpp:57:14:57:16 | buf | semmle.label | buf |
| test.cpp:61:9:61:19 | access to array | semmle.label | access to array |
| test.cpp:61:14:61:16 | buf | semmle.label | buf |
| test.cpp:66:32:66:32 | p | semmle.label | p |
| test.cpp:66:32:66:32 | p | semmle.label | p |
| test.cpp:70:33:70:33 | p | semmle.label | p |
| test.cpp:71:5:71:17 | access to array | semmle.label | access to array |
| test.cpp:72:5:72:15 | access to array | semmle.label | access to array |
| test.cpp:76:26:76:46 | & ... | semmle.label | & ... |
| test.cpp:76:32:76:34 | buf | semmle.label | buf |
| test.cpp:77:26:77:44 | & ... | semmle.label | & ... |
| test.cpp:77:32:77:34 | buf | semmle.label | buf |
| test.cpp:79:27:79:34 | buf | semmle.label | buf |
| test.cpp:79:32:79:34 | buf | semmle.label | buf |
| test.cpp:85:34:85:36 | buf | semmle.label | buf |
| test.cpp:87:5:87:31 | access to array | semmle.label | access to array |
| test.cpp:88:5:88:27 | access to array | semmle.label | access to array |
| test.cpp:96:13:96:15 | arr | semmle.label | arr |
| test.cpp:96:13:96:18 | access to array | semmle.label | access to array |
| test.cpp:111:17:111:19 | arr | semmle.label | arr |
| test.cpp:111:17:111:22 | access to array | semmle.label | access to array |
| test.cpp:115:35:115:37 | arr | semmle.label | arr |
| test.cpp:115:35:115:40 | access to array | semmle.label | access to array |
| test.cpp:119:17:119:19 | arr | semmle.label | arr |
| test.cpp:119:17:119:22 | access to array | semmle.label | access to array |
| test.cpp:128:9:128:11 | arr | semmle.label | arr |
| test.cpp:128:9:128:14 | access to array | semmle.label | access to array |
| test.cpp:134:25:134:27 | arr | semmle.label | arr |
| test.cpp:136:9:136:16 | ... += ... | semmle.label | ... += ... |
| test.cpp:138:13:138:15 | arr | semmle.label | arr |
| test.cpp:143:18:143:21 | asdf | semmle.label | asdf |
| test.cpp:143:18:143:21 | asdf | semmle.label | asdf |
| test.cpp:148:23:148:28 | buffer | semmle.label | buffer |
| test.cpp:150:5:150:11 | access to array | semmle.label | access to array |
| test.cpp:151:5:151:11 | access to array | semmle.label | access to array |
| test.cpp:159:25:159:29 | array | semmle.label | array |
| test.cpp:161:5:161:10 | access to array | semmle.label | access to array |
| test.cpp:162:5:162:10 | access to array | semmle.label | access to array |
subpaths
#select
| test.cpp:35:5:35:22 | PointerAdd: access to array | test.cpp:35:10:35:12 | buf | test.cpp:35:5:35:22 | access to array | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:35:5:35:26 | Store: ... = ... | write |
Expand All @@ -61,5 +108,8 @@ subpaths
| test.cpp:61:9:61:19 | PointerAdd: access to array | test.cpp:61:14:61:16 | buf | test.cpp:61:9:61:19 | access to array | This pointer arithmetic may have an off-by-2 error allowing it to overrun $@ at this $@. | test.cpp:19:9:19:11 | buf | buf | test.cpp:61:9:61:23 | Store: ... = ... | write |
| test.cpp:72:5:72:15 | PointerAdd: access to array | test.cpp:79:32:79:34 | buf | test.cpp:72:5:72:15 | access to array | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:72:5:72:19 | Store: ... = ... | write |
| test.cpp:77:27:77:44 | PointerAdd: access to array | test.cpp:77:32:77:34 | buf | test.cpp:66:32:66:32 | p | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:67:5:67:10 | Store: ... = ... | write |
| test.cpp:88:5:88:27 | PointerAdd: access to array | test.cpp:85:34:85:36 | buf | test.cpp:88:5:88:27 | access to array | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:88:5:88:31 | Store: ... = ... | write |
| test.cpp:128:9:128:14 | PointerAdd: access to array | test.cpp:128:9:128:11 | arr | test.cpp:128:9:128:14 | access to array | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:125:11:125:13 | arr | arr | test.cpp:128:9:128:18 | Store: ... = ... | write |
| test.cpp:136:9:136:16 | PointerAdd: ... += ... | test.cpp:143:18:143:21 | asdf | test.cpp:138:13:138:15 | arr | This pointer arithmetic may have an off-by-2 error allowing it to overrun $@ at this $@. | test.cpp:142:10:142:13 | asdf | asdf | test.cpp:138:12:138:15 | Load: * ... | read |
| test.cpp:151:5:151:11 | PointerAdd: access to array | test.cpp:148:23:148:28 | buffer | test.cpp:151:5:151:11 | access to array | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:147:19:147:24 | buffer | buffer | test.cpp:151:5:151:15 | Store: ... = ... | write |
| test.cpp:162:5:162:10 | PointerAdd: access to array | test.cpp:159:25:159:29 | array | test.cpp:162:5:162:10 | access to array | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:158:10:158:14 | array | array | test.cpp:162:5:162:19 | Store: ... = ... | write |
27 changes: 27 additions & 0 deletions cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/constant-size/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,3 +142,30 @@ void testStrncmp1() {
char asdf[5];
testStrncmp2(asdf);
}

void pointer_size_larger_than_array_element_size() {
unsigned char buffer[100]; // getByteSize() = 100
int *ptr = (int *)buffer; // pai.getElementSize() will be sizeof(int) = 4 -> size = 25

ptr[24] = 0; // GOOD: writes bytes 96, 97, 98, 99
ptr[25] = 0; // BAD: writes bytes 100, 101, 102, 103
}

struct vec2 { int x, y; };
struct vec3 { int x, y, z; };

void pointer_size_smaller_than_array_element_size_but_does_not_divide_it() {
vec3 array[3]; // getByteSize() = 9 * sizeof(int)
vec2 *ptr = (vec2 *)array; // pai.getElementSize() will be 2 * sizeof(int) -> size = 4

ptr[3] = vec2{}; // GOOD: writes ints 6, 7
ptr[4] = vec2{}; // BAD: writes ints 8, 9
}

void pointer_size_larger_than_array_element_size_and_does_not_divide_it() {
vec2 array[2]; // getByteSize() = 4 * sizeof(int) = 4 * 4 = 16
vec3 *ptr = (vec3 *)array; // pai.getElementSize() will be 3 * sizeof(int) -> size = 1

ptr[0] = vec3{}; // GOOD: writes ints 0, 1, 2
ptr[1] = vec3{}; // BAD: writes ints 3, 4, 5 [NOT DETECTED]
}