Skip to content

Commit

Permalink
Updating test cases.
Browse files Browse the repository at this point in the history
  • Loading branch information
bdrodes committed Sep 5, 2024
1 parent 8ab22fe commit 1005a89
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 19 deletions.
7 changes: 6 additions & 1 deletion cpp/ql/lib/semmle/code/cpp/security/BufferAccess.qll
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ abstract class BufferAccess extends Expr {
* - 1 = buffer range [0, getSize) is accessed entirely.
* - 2 = buffer range [0, getSize) may be accessed partially or entirely.
* - 3 = buffer is accessed at offset getSize - 1.
* - 4 = buffer is accessed with null terminator read protections
* (does not read past null terminator, regardless of access size)
*/
abstract Expr getBuffer(string bufferDesc, int accessType);

Expand Down Expand Up @@ -129,7 +131,10 @@ class StrncpyBA extends BufferAccess {
result = this.(FunctionCall).getArgument(0) and
bufferDesc = "destination buffer" and
accessType = 2
// NOTE, ignoring getting the source buffer (arg 1) since reading past the the source null terminator is not the behavior of strncpy
or
result = this.(FunctionCall).getArgument(1) and
bufferDesc = "source buffer" and
accessType = 4
}

override Expr getSizeExpr() { result = this.(FunctionCall).getArgument(2) }
Expand Down
4 changes: 3 additions & 1 deletion cpp/ql/src/Critical/OverflowStatic.ql
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ class BufferAccess extends ArrayExpr {
not exists(Macro m |
m.getName() = "strcmp" and
m.getAnInvocation().getAnExpandedElement() = this
)
) and
//A buffer access must be reachable (not in dead code)
reachable(this)
}

int bufferSize() { staticBuffer(this.getArrayBase(), _, result) }
Expand Down
1 change: 1 addition & 0 deletions cpp/ql/src/Security/CWE/CWE-119/OverflowBuffer.ql
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ from
BufferAccess ba, string bufferDesc, int accessSize, int accessType, Element bufferAlloc,
int bufferSize, string message
where
accessType != 4 and
accessSize = ba.getSize() and
bufferSize = getBufferSize(ba.getBuffer(bufferDesc, accessType), bufferAlloc) and
(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ edges
| main.cpp:9:29:9:32 | *argv | tests_restrict.c:15:41:15:44 | *argv | provenance | |
| main.cpp:9:29:9:32 | tests_restrict_main output argument | main.cpp:10:20:10:23 | **argv | provenance | |
| main.cpp:9:29:9:32 | tests_restrict_main output argument | main.cpp:10:20:10:23 | *argv | provenance | |
| main.cpp:10:20:10:23 | **argv | tests.cpp:688:32:688:35 | **argv | provenance | |
| main.cpp:10:20:10:23 | *argv | tests.cpp:688:32:688:35 | **argv | provenance | |
| main.cpp:10:20:10:23 | *argv | tests.cpp:688:32:688:35 | *argv | provenance | |
| main.cpp:10:20:10:23 | **argv | tests.cpp:689:32:689:35 | **argv | provenance | |
| main.cpp:10:20:10:23 | *argv | tests.cpp:689:32:689:35 | **argv | provenance | |
| main.cpp:10:20:10:23 | *argv | tests.cpp:689:32:689:35 | *argv | provenance | |
| overflowdestination.cpp:23:45:23:48 | **argv | overflowdestination.cpp:23:45:23:48 | **argv | provenance | |
| overflowdestination.cpp:23:45:23:48 | **argv | overflowdestination.cpp:23:45:23:48 | *argv | provenance | |
| test_buffer_overrun.cpp:32:46:32:49 | **argv | test_buffer_overrun.cpp:32:46:32:49 | **argv | provenance | |
Expand All @@ -46,12 +46,12 @@ edges
| tests.cpp:628:14:628:14 | *s [*home] | tests.cpp:628:14:628:19 | *home | provenance | |
| tests.cpp:628:14:628:14 | *s [*home] | tests.cpp:628:16:628:19 | *home | provenance | |
| tests.cpp:628:16:628:19 | *home | tests.cpp:628:14:628:19 | *home | provenance | |
| tests.cpp:688:32:688:35 | **argv | tests.cpp:713:9:713:15 | *access to array | provenance | |
| tests.cpp:688:32:688:35 | **argv | tests.cpp:714:9:714:15 | *access to array | provenance | |
| tests.cpp:688:32:688:35 | *argv | tests.cpp:713:9:713:15 | *access to array | provenance | |
| tests.cpp:688:32:688:35 | *argv | tests.cpp:714:9:714:15 | *access to array | provenance | |
| tests.cpp:713:9:713:15 | *access to array | tests.cpp:613:19:613:24 | *source | provenance | |
| tests.cpp:714:9:714:15 | *access to array | tests.cpp:622:19:622:24 | *source | provenance | |
| tests.cpp:689:32:689:35 | **argv | tests.cpp:714:9:714:15 | *access to array | provenance | |
| tests.cpp:689:32:689:35 | **argv | tests.cpp:715:9:715:15 | *access to array | provenance | |
| tests.cpp:689:32:689:35 | *argv | tests.cpp:714:9:714:15 | *access to array | provenance | |
| tests.cpp:689:32:689:35 | *argv | tests.cpp:715:9:715:15 | *access to array | provenance | |
| tests.cpp:714:9:714:15 | *access to array | tests.cpp:613:19:613:24 | *source | provenance | |
| tests.cpp:715:9:715:15 | *access to array | tests.cpp:622:19:622:24 | *source | provenance | |
| tests_restrict.c:15:41:15:44 | **argv | tests_restrict.c:15:41:15:44 | **argv | provenance | |
| tests_restrict.c:15:41:15:44 | *argv | tests_restrict.c:15:41:15:44 | *argv | provenance | |
nodes
Expand Down Expand Up @@ -85,10 +85,10 @@ nodes
| tests.cpp:628:14:628:14 | *s [*home] | semmle.label | *s [*home] |
| tests.cpp:628:14:628:19 | *home | semmle.label | *home |
| tests.cpp:628:16:628:19 | *home | semmle.label | *home |
| tests.cpp:688:32:688:35 | **argv | semmle.label | **argv |
| tests.cpp:688:32:688:35 | *argv | semmle.label | *argv |
| tests.cpp:713:9:713:15 | *access to array | semmle.label | *access to array |
| tests.cpp:689:32:689:35 | **argv | semmle.label | **argv |
| tests.cpp:689:32:689:35 | *argv | semmle.label | *argv |
| tests.cpp:714:9:714:15 | *access to array | semmle.label | *access to array |
| tests.cpp:715:9:715:15 | *access to array | semmle.label | *access to array |
| tests_restrict.c:15:41:15:44 | **argv | semmle.label | **argv |
| tests_restrict.c:15:41:15:44 | **argv | semmle.label | **argv |
| tests_restrict.c:15:41:15:44 | *argv | semmle.label | *argv |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -654,17 +654,18 @@ void test26(bool cond)
if (ptr[-1] == 0) { return; } // GOOD: accesses buffer[1]
}

#define IND 100
#define MAX_SIZE 100
void test27(){
char *src = "";
char *dest = "abcdefgh";
const int size = 100;
int ind = 100;
char buffer[size];
char buffer[MAX_SIZE];

strncpy(dest, src, strlen(dest)); // GOOD, strncpy will not read past null terminator of source
strncpy(dest, src, 8); // GOOD, strncpy will not read past null terminator of source

if(ind < size){
buffer[ind] = 0; // GOOD: out of bounds, but inaccessible code
if(IND < MAX_SIZE){
buffer[IND] = 0; // GOOD: out of bounds, but inaccessible code
}
}

Expand Down

0 comments on commit 1005a89

Please sign in to comment.