Skip to content

Commit

Permalink
Merge pull request #17548 from hvitved/shared/inline-test-post-process
Browse files Browse the repository at this point in the history
Shared: Post-processing query for inline test expectations
  • Loading branch information
hvitved authored Oct 31, 2024
2 parents 977eb05 + 495c92d commit 2b37c6c
Show file tree
Hide file tree
Showing 72 changed files with 1,651 additions and 717 deletions.
28 changes: 1 addition & 27 deletions cpp/ql/test/TestUtilities/InlineExpectationsTest.qll
Original file line number Diff line number Diff line change
Expand Up @@ -5,31 +5,5 @@

import cpp as C
private import codeql.util.test.InlineExpectationsTest

private module Impl implements InlineExpectationsTestSig {
private newtype TExpectationComment = MkExpectationComment(C::CppStyleComment c)

/**
* A class representing a line comment in the CPP style.
* Unlike the `CppStyleComment` class, however, the string returned by `getContents` does _not_
* include the preceding comment marker (`//`).
*/
class ExpectationComment extends TExpectationComment {
C::CppStyleComment comment;

ExpectationComment() { this = MkExpectationComment(comment) }

/** Returns the contents of the given comment, _without_ the preceding comment marker (`//`). */
string getContents() { result = comment.getContents().suffix(2) }

/** Gets a textual representation of this element. */
string toString() { result = comment.toString() }

/** Gets the location of this comment. */
Location getLocation() { result = comment.getLocation() }
}

class Location = C::Location;
}

private import internal.InlineExpectationsTestImpl
import Make<Impl>
21 changes: 21 additions & 0 deletions cpp/ql/test/TestUtilities/InlineExpectationsTestQuery.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* @kind test-postprocess
*/

private import cpp
private import codeql.util.test.InlineExpectationsTest as T
private import internal.InlineExpectationsTestImpl
import T::TestPostProcessing
import T::TestPostProcessing::Make<Impl, Input>

private module Input implements T::TestPostProcessing::InputSig<Impl> {
string getRelativeUrl(Location location) {
exists(File f, int startline, int startcolumn, int endline, int endcolumn |
location.hasLocationInfo(_, startline, startcolumn, endline, endcolumn) and
f = location.getFile()
|
result =
f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn
)
}
}
28 changes: 28 additions & 0 deletions cpp/ql/test/TestUtilities/internal/InlineExpectationsTestImpl.qll
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import cpp as C
private import codeql.util.test.InlineExpectationsTest

module Impl implements InlineExpectationsTestSig {
private newtype TExpectationComment = MkExpectationComment(C::CppStyleComment c)

/**
* A class representing a line comment in the CPP style.
* Unlike the `CppStyleComment` class, however, the string returned by `getContents` does _not_
* include the preceding comment marker (`//`).
*/
class ExpectationComment extends TExpectationComment {
C::CppStyleComment comment;

ExpectationComment() { this = MkExpectationComment(comment) }

/** Returns the contents of the given comment, _without_ the preceding comment marker (`//`). */
string getContents() { result = comment.getContents().suffix(2) }

/** Gets a textual representation of this element. */
string toString() { result = comment.toString() }

/** Gets the location of this comment. */
Location getLocation() { result = comment.getLocation() }
}

class Location = C::Location;
}
3 changes: 2 additions & 1 deletion cpp/ql/test/query-tests/Critical/SizeCheck/SizeCheck.qlref
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
Critical/SizeCheck.ql
query: Critical/SizeCheck.ql
postprocess: TestUtilities/InlineExpectationsTestQuery.ql
14 changes: 7 additions & 7 deletions cpp/ql/test/query-tests/Critical/SizeCheck/test.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ void free(void *ptr);

void bad0(void) {

float *fptr = malloc(3); // BAD -- Too small
double *dptr = malloc(5); // BAD -- Too small
float *fptr = malloc(3); // $ Alert -- Too small
double *dptr = malloc(5); // $ Alert -- Too small
free(fptr);
free(dptr);
}
Expand All @@ -29,8 +29,8 @@ void good0(void) {

void bad1(void) {

float *fptr = malloc(sizeof(short)); // BAD -- Too small
double *dptr = malloc(sizeof(float)); // BAD -- Too small
float *fptr = malloc(sizeof(short)); // $ Alert -- Too small
double *dptr = malloc(sizeof(float)); // $ Alert -- Too small
free(fptr);
free(dptr);
}
Expand All @@ -56,7 +56,7 @@ typedef union _myUnion

void test_union() {
MyUnion *a = malloc(sizeof(MyUnion)); // GOOD
MyUnion *b = malloc(sizeof(MyStruct)); // BAD (too small)
MyUnion *b = malloc(sizeof(MyStruct)); // $ Alert (too small)
}

// --- custom allocators ---
Expand All @@ -66,6 +66,6 @@ void *MyMalloc2(size_t size);

void customAllocatorTests()
{
float *fptr1 = MyMalloc1(3); // BAD (too small) [NOT DETECTED]
float *fptr2 = MyMalloc2(3); // BAD (too small) [NOT DETECTED]
float *fptr1 = MyMalloc1(3); // $ MISSING: BAD (too small)
float *fptr2 = MyMalloc2(3); // $ MISSING: BAD (too small)
}
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
Security/CWE/CWE-022/TaintedPath.ql
query: Security/CWE/CWE-022/TaintedPath.ql
postprocess: TestUtilities/InlineExpectationsTestQuery.ql
34 changes: 17 additions & 17 deletions cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/test.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#define PATH_MAX 4096
///// Test code /////

int main(int argc, char** argv) {
int main(int argc, char** argv) { // $ Source=argv
char *userAndFile = argv[2];

{
Expand All @@ -14,7 +14,7 @@ int main(int argc, char** argv) {
size_t len = strlen(fileName);
strncat(fileName+len, userAndFile, FILENAME_MAX-len-1);
// BAD: a string from the user is used in a filename
fopen(fileName, "wb+");
fopen(fileName, "wb+"); // $ Alert=argv
}

{
Expand All @@ -29,30 +29,30 @@ int main(int argc, char** argv) {

{
char *fileName = argv[1];
fopen(fileName, "wb+"); // BAD
fopen(fileName, "wb+"); // $ Alert=argv
}

{
char fileName[20];
scanf("%s", fileName);
fopen(fileName, "wb+"); // BAD
scanf("%s", fileName); // $ Source=scanf_output1
fopen(fileName, "wb+"); // $ Alert=scanf_output1
}

{
char *fileName = (char*)malloc(20 * sizeof(char));
scanf("%s", fileName);
fopen(fileName, "wb+"); // BAD
scanf("%s", fileName); // $ Source=scanf_output2
fopen(fileName, "wb+"); // $ Alert=scanf_output2
}

{
char *tainted = getenv("A_STRING");
fopen(tainted, "wb+"); // BAD
char *tainted = getenv("A_STRING"); // $ Source=getenv1
fopen(tainted, "wb+"); // $ Alert=getenv1
}

{
char buffer[1024];
strncpy(buffer, getenv("A_STRING"), 1024);
fopen(buffer, "wb+"); // BAD
strncpy(buffer, getenv("A_STRING"), 1024); // $ Source=getenv2
fopen(buffer, "wb+"); // $ Alert=getenv2
fopen(buffer, "wb+"); // (we don't want a duplicate result here)
}

Expand All @@ -66,22 +66,22 @@ int main(int argc, char** argv) {

{
void readFile(const char *fileName);
readFile(argv[1]); // BAD
readFile(argv[1]); // $ Alert=argv
}

{
char buffer[1024];
read(0, buffer, 1024);
read(0, buffer, 1024);
fopen(buffer, "wb+"); // BAD [duplicated with both sources]
read(0, buffer, 1024); // $ Source=read_output1
read(0, buffer, 1024); // $ Source=read_output2
fopen(buffer, "wb+"); // $ SPURIOUS: Alert=read_output1 $ Alert=read_output2 [duplicated with both sources]
}

{
char *userAndFile = argv[2];
char fileBuffer[PATH_MAX];
snprintf(fileBuffer, sizeof(fileBuffer), "/home/%s", userAndFile);
// BAD: a string from the user is used in a filename
fopen(fileBuffer, "wb+");
fopen(fileBuffer, "wb+"); // $ Alert=argv
}

{
Expand All @@ -95,7 +95,7 @@ int main(int argc, char** argv) {
char fileBuffer[PATH_MAX];
snprintf(fileBuffer, sizeof(fileBuffer), "/home/user/files/%s", fileName);
// GOOD: We know that the filename is safe and stays within the public folder. But we currently get an FP here.
FILE *file = fopen(fileBuffer, "wb+");
FILE *file = fopen(fileBuffer, "wb+"); // $ SPURIOUS: Alert=argv
}

{
Expand Down
2 changes: 1 addition & 1 deletion csharp/ql/test/TestUtilities/InlineExpectationsTest.qll
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Inline expectation tests for CSharp.
* Inline expectation tests for C#.
* See `shared/util/codeql/util/test/InlineExpectationsTest.qll`
*/

Expand Down
21 changes: 21 additions & 0 deletions csharp/ql/test/TestUtilities/InlineExpectationsTestQuery.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* @kind test-postprocess
*/

private import csharp
private import codeql.util.test.InlineExpectationsTest as T
private import internal.InlineExpectationsTestImpl
import T::TestPostProcessing
import T::TestPostProcessing::Make<Impl, Input>

private module Input implements T::TestPostProcessing::InputSig<Impl> {
string getRelativeUrl(Location location) {
exists(File f, int startline, int startcolumn, int endline, int endcolumn |
location.hasLocationInfo(_, startline, startcolumn, endline, endcolumn) and
f = location.getFile()
|
result =
f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn
)
}
}
4 changes: 0 additions & 4 deletions csharp/ql/test/TestUtilities/PrettyPrintModels.ql
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,3 @@
import semmle.code.csharp.dataflow.internal.ExternalFlow
import codeql.dataflow.test.ProvenancePathGraph
import codeql.dataflow.test.ProvenancePathGraph::TestPostProcessing::TranslateProvenanceResults<interpretModelForTest/2>

from string relation, int row, int column, string data
where results(relation, row, column, data)
select relation, row, column, data
82 changes: 82 additions & 0 deletions csharp/ql/test/TestUtilities/inline-tests/InlineTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
class C
{
void Problems()
{
// correct expectation comment, but only for `problem-query`
var x = "Alert"; // $ Alert

// irrelevant expectation comment, will be ignored
x = "Not an alert"; // $ IrrelevantTag

// incorrect expectation comment
x = "Also not an alert"; // $ Alert

// missing expectation comment, but only for `problem-query`
x = "Alert";

// correct expectation comment
x = "Alert"; // $ Alert[problem-query]
}

void PathProblems()
{
// correct expectation comments, but only for `path-problem-query`
var source = "Source"; // $ Source
var sink = "Sink"; // $ Sink
var x = "Alert:2:1"; // $ Alert

// incorrect expectation comments
source = "Source"; // $ Source
sink = "Sink"; // $ Sink
x = "Not an alert:2:1"; // $ Alert

// missing expectation comments, but only for `path-problem-query`
source = "Source";
sink = "Sink";
x = "Alert:2:1";

// correct expectation comments
source = "Source"; // $ Source[path-problem-query]
sink = "Sink"; // $ Sink[path-problem-query]
x = "Alert:2:1"; // $ Alert[path-problem-query]

// correct expectation comments; the alert location coincides with the sink location
source = "Source"; // $ Source[path-problem-query]
x = "Alert:1:0"; // $ Alert[path-problem-query]

// correct expectation comments; the alert location coincides with the source location
sink = "Sink"; // $ Sink[path-problem-query]
x = "Alert:0:1"; // $ Alert[path-problem-query]

// correct expectation comments, using an identifier tag
source = "Source"; // $ Source[path-problem-query]=source1
sink = "Sink"; // $ Sink[path-problem-query]=source1
x = "Alert:2:1"; // $ Alert[path-problem-query]=source1

// incorrect expectation comment, using wrong identifier tag at the sink
source = "Source"; // $ Source[path-problem-query]=source2
sink = "Sink"; // $ Sink[path-problem-query]=source1
x = "Alert:2:1"; // $ Alert[path-problem-query]=source2

// incorrect expectation comment, using wrong identifier tag at the alert
source = "Source"; // $ Source[path-problem-query]=source3
sink = "Sink"; // $ Sink[path-problem-query]=source3
x = "Alert:2:1"; // $ Alert[path-problem-query]=source2

// correct expectation comments, using an identifier tag; the alert location coincides with the sink location
source = "Source"; // $ Source[path-problem-query]=source4
x = "Alert:1:0"; // $ Alert[path-problem-query]=source4

// incorrect expectation comments, using an identifier tag; the alert location coincides with the sink location
source = "Source"; // $ Source[path-problem-query]=source5
x = "Alert:1:0"; // $ Alert[path-problem-query]=source4

// correct expectation comments, using an identifier tag; the alert location coincides with the source location
sink = "Sink"; // $ Sink[path-problem-query]=sink1
x = "Alert:0:1"; // $ Alert[path-problem-query]=sink1

// incorrect expectation comments, using an identifier tag; the alert location coincides with the source location
sink = "Sink"; // $ Sink[path-problem-query]=sink2
x = "Alert:0:1"; // $ Alert[path-problem-query]=sink1
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#select
| InlineTests.cs:26:17:26:27 | "Alert:2:1" | InlineTests.cs:24:22:24:29 | "Source" | InlineTests.cs:25:20:25:25 | "Sink" | This is a problem |
| InlineTests.cs:36:13:36:23 | "Alert:2:1" | InlineTests.cs:34:18:34:25 | "Source" | InlineTests.cs:35:16:35:21 | "Sink" | This is a problem |
| InlineTests.cs:41:13:41:23 | "Alert:2:1" | InlineTests.cs:39:18:39:25 | "Source" | InlineTests.cs:40:16:40:21 | "Sink" | This is a problem |
| InlineTests.cs:45:13:45:23 | "Alert:1:0" | InlineTests.cs:44:18:44:25 | "Source" | InlineTests.cs:45:13:45:23 | "Alert:1:0" | This is a problem |
| InlineTests.cs:49:13:49:23 | "Alert:0:1" | InlineTests.cs:49:13:49:23 | "Alert:0:1" | InlineTests.cs:48:16:48:21 | "Sink" | This is a problem |
| InlineTests.cs:54:13:54:23 | "Alert:2:1" | InlineTests.cs:52:18:52:25 | "Source" | InlineTests.cs:53:16:53:21 | "Sink" | This is a problem |
| InlineTests.cs:59:13:59:23 | "Alert:2:1" | InlineTests.cs:57:18:57:25 | "Source" | InlineTests.cs:58:16:58:21 | "Sink" | This is a problem |
| InlineTests.cs:64:13:64:23 | "Alert:2:1" | InlineTests.cs:62:18:62:25 | "Source" | InlineTests.cs:63:16:63:21 | "Sink" | This is a problem |
| InlineTests.cs:68:13:68:23 | "Alert:1:0" | InlineTests.cs:67:18:67:25 | "Source" | InlineTests.cs:68:13:68:23 | "Alert:1:0" | This is a problem |
| InlineTests.cs:72:13:72:23 | "Alert:1:0" | InlineTests.cs:71:18:71:25 | "Source" | InlineTests.cs:72:13:72:23 | "Alert:1:0" | This is a problem |
| InlineTests.cs:76:13:76:23 | "Alert:0:1" | InlineTests.cs:76:13:76:23 | "Alert:0:1" | InlineTests.cs:75:16:75:21 | "Sink" | This is a problem |
| InlineTests.cs:80:13:80:23 | "Alert:0:1" | InlineTests.cs:80:13:80:23 | "Alert:0:1" | InlineTests.cs:79:16:79:21 | "Sink" | This is a problem |
edges
testFailures
| InlineTests.cs:6:26:6:35 | // ... | Missing result: Alert |
| InlineTests.cs:12:34:12:43 | // ... | Missing result: Alert |
| InlineTests.cs:29:28:29:38 | // ... | Missing result: Source |
| InlineTests.cs:30:24:30:32 | // ... | Missing result: Sink |
| InlineTests.cs:31:33:31:42 | // ... | Missing result: Alert |
| InlineTests.cs:34:18:34:25 | "Source" | Unexpected result: Source |
| InlineTests.cs:35:16:35:21 | "Sink" | Unexpected result: Sink |
| InlineTests.cs:36:13:36:23 | InlineTests.cs:34:18:34:25 | Unexpected result: Alert |
| InlineTests.cs:58:16:58:21 | "Sink" | Unexpected result: Sink=source2 |
| InlineTests.cs:58:24:58:60 | // ... | Missing result: Sink[path-problem-query]=source1 |
| InlineTests.cs:64:13:64:23 | InlineTests.cs:62:18:62:25 | Unexpected result: Alert=source3 |
| InlineTests.cs:64:26:64:63 | // ... | Missing result: Alert[path-problem-query]=source2 |
| InlineTests.cs:72:13:72:23 | "Alert:1:0" | Unexpected result: Alert=source5 |
| InlineTests.cs:72:26:72:63 | // ... | Missing result: Alert[path-problem-query]=source4 |
| InlineTests.cs:79:16:79:21 | "Sink" | Unexpected result: Sink=sink1 |
| InlineTests.cs:79:24:79:58 | // ... | Missing result: Sink[path-problem-query]=sink2 |
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
query: TestUtilities/inline-tests/queries/PathProblemQuery.ql
postprocess: TestUtilities/InlineExpectationsTestQuery.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#select
| InlineTests.cs:6:17:6:23 | "Alert" | This is a problem |
| InlineTests.cs:15:13:15:19 | "Alert" | This is a problem |
| InlineTests.cs:18:13:18:19 | "Alert" | This is a problem |
testFailures
| InlineTests.cs:12:34:12:43 | // ... | Missing result: Alert |
| InlineTests.cs:15:13:15:19 | This is a problem | Unexpected result: Alert |
| InlineTests.cs:26:30:26:39 | // ... | Missing result: Alert |
| InlineTests.cs:31:33:31:42 | // ... | Missing result: Alert |
Loading

0 comments on commit 2b37c6c

Please sign in to comment.