Skip to content

Implement OutOfBounds package #281

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.

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
merged 29 commits into from
Apr 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
8539f46
OutOfBounds: WIP libraries
Mar 30, 2023
8c30b0f
Update OutOfBounds.qll
Mar 30, 2023
11406b1
Update OutOfBounds.qll
Mar 30, 2023
667fc33
Update OutOfBounds.qll
Mar 30, 2023
837b90d
Update OutOfBounds.qll
Mar 30, 2023
4427b40
Add ARR38-C
Mar 30, 2023
dfda651
ARR38-C: Update query
Mar 30, 2023
5f8a6f7
Implement OutOfBounds.qll, ARR38-C, and ARR30-C
Apr 5, 2023
d28b90f
Merge branch 'main' into OutOfBounds
Apr 5, 2023
4766e4d
Correct OutOfBounds.qll format
Apr 5, 2023
ba41544
Merge branch 'OutOfBounds' of https://github.com/kraiouchkine/codeql-…
Apr 5, 2023
2696ef9
Update DoNotFormOutOfBoundsPointersOrArraySubscripts.md
Apr 5, 2023
7f766cb
Fix strncat/wcscat param definition and add rules
Apr 5, 2023
406dab5
Add RULE-21-18 test-case
Apr 5, 2023
883ecca
Update RULE-21-17 and RULE-21-18 tests
Apr 5, 2023
e9dd4a8
Update strtok param indices in OutOfBounds library
Apr 5, 2023
76146e4
Refactor OutOfBounds.qll and arg/offset model
Apr 6, 2023
1aa1766
Add test case to ARR30-C
Apr 6, 2023
94a05c4
Correct OutOfBounds.qll formatting
Apr 6, 2023
0f6e747
Resolve performance issue in OutOfBounds.qll
Apr 6, 2023
3b8361a
Expand ARR30-C coverage and test + add comments
Apr 6, 2023
8ef1af9
Fix OutOfBounds.qll format
Apr 6, 2023
b3523be
ARR38-C: Add missing GVN logic
Apr 6, 2023
a8c7dc3
Restrict ARR30-C to reduce FPs and fix performance
Apr 7, 2023
9dd30f0
Remove defining argument nodes from data-flow graph
Apr 7, 2023
2f31431
Correct ARR30-C metadata
Apr 7, 2023
aa4ef08
Merge branch 'main' into OutOfBounds
Apr 7, 2023
03c17ae
Update getNameOrInternalName regex
Apr 7, 2023
ea9b0f4
Merge branch 'main' into OutOfBounds
rvermeulen Apr 7, 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
1 change: 1 addition & 0 deletions .vscode/tasks.json
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@
"Null",
"OperatorInvariants",
"Operators",
"OutOfBounds",
"Pointers",
"Pointers1",
"Pointers2",
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/**
* @id c/cert/do-not-form-out-of-bounds-pointers-or-array-subscripts
* @name ARR30-C: Do not form or use out-of-bounds pointers or array subscripts
* @description Forming or using an out-of-bounds pointer is undefined behavior and can result in
* invalid memory accesses.
* @kind problem
* @precision medium
* @problem.severity error
* @tags external/cert/id/arr30-c
* correctness
* security
* external/cert/obligation/rule
*/

import cpp
import codingstandards.c.cert
import codingstandards.c.OutOfBounds

from
OOB::BufferAccess ba, Expr bufferArg, Expr sizeArg, OOB::PointerToObjectSource bufferSource,
string message
where
not isExcluded(ba, OutOfBoundsPackage::doNotFormOutOfBoundsPointersOrArraySubscriptsQuery()) and
// exclude loops
not exists(Loop loop | loop.getStmt().getChildStmt*() = ba.getEnclosingStmt()) and
// exclude size arguments that are of type ssize_t
not sizeArg.getAChild*().(VariableAccess).getTarget().getType() instanceof Ssize_t and
// exclude size arguments that are assigned the result of a function call e.g. ftell
not sizeArg.getAChild*().(VariableAccess).getTarget().getAnAssignedValue() instanceof FunctionCall and
// exclude field or array accesses for the size arguments
not sizeArg.getAChild*() instanceof FieldAccess and
not sizeArg.getAChild*() instanceof ArrayExpr and
(
exists(int sizeArgValue, int bufferArgSize |
OOB::isSizeArgGreaterThanBufferSize(bufferArg, sizeArg, bufferSource, bufferArgSize, sizeArgValue, ba) and
message =
"Buffer accesses offset " + sizeArgValue +
" which is greater than the fixed size " + bufferArgSize + " of the $@."
)
or
exists(int sizeArgUpperBound, int sizeMult, int bufferArgSize |
OOB::isSizeArgNotCheckedLessThanFixedBufferSize(bufferArg, sizeArg, bufferSource,
bufferArgSize, ba, sizeArgUpperBound, sizeMult) and
message =
"Buffer may access up to offset " + sizeArgUpperBound + "*" + sizeMult +
" which is greater than the fixed size " + bufferArgSize + " of the $@."
)
or
OOB::isSizeArgNotCheckedGreaterThanZero(bufferArg, sizeArg, bufferSource, ba) and
message = "Buffer access may be to a negative index in the buffer."
)
select ba, message, bufferSource, "buffer"
486 changes: 486 additions & 0 deletions c/cert/src/rules/ARR38-C/LibraryFunctionArgumentOutOfBounds.md

Large diffs are not rendered by default.

25 changes: 25 additions & 0 deletions c/cert/src/rules/ARR38-C/LibraryFunctionArgumentOutOfBounds.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/**
* @id c/cert/library-function-argument-out-of-bounds
* @name ARR38-C: Guarantee that library functions do not form invalid pointers
* @description Passing out-of-bounds pointers or erroneous size arguments to standard library
* functions can result in out-of-bounds accesses and other undefined behavior.
* @kind problem
* @precision high
* @problem.severity error
* @tags external/cert/id/arr38-c
* correctness
* security
* external/cert/obligation/rule
*/

import cpp
import codingstandards.c.cert
import codingstandards.c.OutOfBounds

from
OOB::BufferAccessLibraryFunctionCall fc, string message, Expr bufferArg, string bufferArgStr,
Expr sizeOrOtherBufferArg, string otherStr
where
not isExcluded(fc, OutOfBoundsPackage::libraryFunctionArgumentOutOfBoundsQuery()) and
OOB::problems(fc, message, bufferArg, bufferArgStr, sizeOrOtherBufferArg, otherStr)
select fc, message, bufferArg, bufferArgStr, sizeOrOtherBufferArg, otherStr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
| test.c:8:3:8:11 | ... + ... | Buffer accesses offset 404 which is greater than the fixed size 400 of the $@. | test.c:8:3:8:5 | arr | buffer |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| test.c:63:3:63:9 | access to array | Buffer access may be to a negative index in the buffer. | test.c:63:3:63:5 | arr | buffer |
| test.c:65:3:65:9 | access to array | Buffer accesses offset 44 which is greater than the fixed size 40 of the $@. | test.c:65:3:65:5 | arr | buffer |
| test.c:66:3:66:10 | access to array | Buffer access may be to a negative index in the buffer. | test.c:66:3:66:5 | arr | buffer |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
rules/ARR30-C/DoNotFormOutOfBoundsPointersOrArraySubscripts.ql
67 changes: 67 additions & 0 deletions c/cert/test/rules/ARR30-C/test.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@


enum { ARRAY_SIZE = 100 };

static int arr[ARRAY_SIZE];

void test_fixed_wrong(void) {
arr + 101; // NON_COMPLIANT
}

void test_fixed_right(void) {
arr + 2; // COMPLIANT
}

void test_no_check(int index) {
arr + index; // NON_COMPLIANT
}

void test_invalid_check(int index) {
if (index < ARRAY_SIZE) {
arr + index; // NON_COMPLIANT - `index` could be negative
}
}

void test_valid_check(int index) {
if (index > 0 && index < ARRAY_SIZE) {
arr + index; // COMPLIANT - `index` cannot be negative
}
}

void test_valid_check_by_type(unsigned int index) {
if (index < ARRAY_SIZE) {
arr + index; // COMPLIANT - `index` cannot be be negative
}
}

void test_local_buffer_invalid_check(int index) {
char buffer[ARRAY_SIZE];

if (index < ARRAY_SIZE) {
char *ptr = buffer + index; // NON_COMPLIANT - `index` could be negative
}

if (index >= 0 && index < ARRAY_SIZE + 2) {
char *ptr = buffer + index; // NON_COMPLIANT - `index` could be too large
}

if (index >= 0 && index < ARRAY_SIZE) {
char *ptr = buffer + index; // COMPLIANT
}
}

void test_dereference_pointer_arithmetic_const(void) {
short ptr16[10];
*(ptr16 - 1); // NON_COMPLIANT - offset is negative
*(ptr16 + 5); // COMPLIANT
*(ptr16 + 11); // NON_COMPLIANT - offset is too large
*(ptr16 - 11); // NON_COMPLIANT - offset is negative
}

void test_array_expr_const(void) {
int arr[10];
arr[-1]; // NON_COMPLIANT - offset is negative
arr[5]; // COMPLIANT
arr[11]; // NON_COMPLIANT - offset is too large
arr[-11]; // NON_COMPLIANT - offset is negative
}
Loading