From e2fe74ccd699a70ed241601af0c9a41a7fa93b10 Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 18 Feb 2025 15:48:54 +0100 Subject: [PATCH 1/8] JS: Add support for RelatedLocation tags --- .../util/test/InlineExpectationsTest.qll | 49 ++++++++++++++----- 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/shared/util/codeql/util/test/InlineExpectationsTest.qll b/shared/util/codeql/util/test/InlineExpectationsTest.qll index 17ac7f1cd257..baa3b2d44404 100644 --- a/shared/util/codeql/util/test/InlineExpectationsTest.qll +++ b/shared/util/codeql/util/test/InlineExpectationsTest.qll @@ -779,22 +779,33 @@ module TestPostProcessing { ) } - private string getTagRegex() { - exists(string sourceSinkTags | - ( - getQueryKind() = "problem" - or - not exists(getSourceTag(_)) and - not exists(getSinkTag(_)) - ) and - sourceSinkTags = "" - or - sourceSinkTags = "|" + getSourceTag(_) + "|" + getSinkTag(_) + bindingset[x, y] + private int exactDivide(int x, int y) { x % y = 0 and result = x / y } + + /** Gets the `n`th related location selected in `row`. */ + private TestLocation getRelatedLocation(int row, int n, string element) { + n >= 0 and + exists(int column | + mainQueryResult(row, column, result) and + queryResults(mainResultSet(), row, column + 1, element) | - result = "(Alert" + sourceSinkTags + ")(\\[(.*)\\])?" + getQueryKind() = "path-problem" and + n = exactDivide(column - 8, 3) + or + getQueryKind() = "problem" and + n = exactDivide(column - 3, 3) ) } + private string getAnActiveTag() { + result = ["Alert", "RelatedLocation"] + or + getQueryKind() = "path-problem" and + result = ["Source", "Sink"] + } + + private string getTagRegex() { result = "(" + concat(getAnActiveTag(), "|") + ")(\\[(.*)\\])?" } + /** * A configuration for matching `// $ Source=foo` comments against actual * path-problem sources. @@ -878,6 +889,18 @@ module TestPostProcessing { not hasPathProblemSink(row, location, _, _) } + private predicate hasRelatedLocation( + int row, TestLocation location, string element, string tag + ) { + getQueryKind() = ["problem", "path-problem"] and + location = getRelatedLocation(row, _, element) and + hasExpectationWithValue("RelatedLocation", _) and + tag = "RelatedLocation" and + not hasAlert(row, location, _, _) and + not hasPathProblemSource(row, location, _, _, _) and + not hasPathProblemSink(row, location, _, _) + } + /** * Gets the expected value for result row `row`, if any. This value must * match the value at the corresponding path-problem source (if it is @@ -899,6 +922,8 @@ module TestPostProcessing { hasPathProblemSink(row, location, element, tag) or hasAlert(row, location, element, tag) + or + hasRelatedLocation(row, location, element, tag) | not exists(getValue(row)) and value = "" or From cd2c4d5e3aec5d5315de3db466088cb9da8d57ba Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 18 Feb 2025 17:25:47 +0100 Subject: [PATCH 2/8] JS: Use post-processed inline test in MissingCsrfMiddleware This query flags the cookie-parsing middleware in order to consolidate huge numbers of alerts into a single alert, which is more manageable. But simply annotating the cookie-parsing middleware with 'Alert' isn't a very useful, we want to annotate which middlewares are vulnerable. --- .../Security/CWE-352/MissingCsrfMiddleware.qlref | 3 ++- .../Security/CWE-352/MissingCsrfMiddlewareBad.js | 14 +++++++------- .../Security/CWE-352/csurf_api_example.js | 4 ++-- .../query-tests/Security/CWE-352/csurf_example.js | 6 +++--- .../test/query-tests/Security/CWE-352/fastify.js | 6 +++--- .../test/query-tests/Security/CWE-352/fastify2.js | 6 +++--- .../query-tests/Security/CWE-352/lusca_example.js | 10 +++++----- .../ql/test/query-tests/Security/CWE-352/tst.js | 6 +++--- .../query-tests/Security/CWE-352/unused_cookies.js | 14 +++++++------- 9 files changed, 35 insertions(+), 34 deletions(-) diff --git a/javascript/ql/test/query-tests/Security/CWE-352/MissingCsrfMiddleware.qlref b/javascript/ql/test/query-tests/Security/CWE-352/MissingCsrfMiddleware.qlref index 37febc7d8f97..6df50f3532d5 100644 --- a/javascript/ql/test/query-tests/Security/CWE-352/MissingCsrfMiddleware.qlref +++ b/javascript/ql/test/query-tests/Security/CWE-352/MissingCsrfMiddleware.qlref @@ -1 +1,2 @@ -Security/CWE-352/MissingCsrfMiddleware.ql +query: Security/CWE-352/MissingCsrfMiddleware.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/javascript/ql/test/query-tests/Security/CWE-352/MissingCsrfMiddlewareBad.js b/javascript/ql/test/query-tests/Security/CWE-352/MissingCsrfMiddlewareBad.js index d2d9b8bd4a16..34217d01c92e 100644 --- a/javascript/ql/test/query-tests/Security/CWE-352/MissingCsrfMiddlewareBad.js +++ b/javascript/ql/test/query-tests/Security/CWE-352/MissingCsrfMiddlewareBad.js @@ -4,17 +4,17 @@ var passport = require('passport'); var app = express(); -app.use(cookieParser()); +app.use(cookieParser()); // $ Alert app.use(passport.authorize({ session: true })); app.post('/changeEmail', function (req, res) { let newEmail = req.cookies["newEmail"]; -}); +}); // $ RelatedLocation (function () { var app = express(); - app.use(cookieParser()); + app.use(cookieParser()); // $ Alert app.use(passport.authorize({ session: true })); const errorCatch = (fn) => @@ -24,13 +24,13 @@ app.post('/changeEmail', function (req, res) { app.post('/changeEmail', errorCatch(async function (req, res) { let newEmail = req.cookies["newEmail"]; - })); + })); // $ RelatedLocation }) (function () { var app = express(); - app.use(cookieParser()); + app.use(cookieParser()); // $ Alert app.use(passport.authorize({ session: true })); const errorCatch = (fn) => @@ -40,9 +40,9 @@ app.post('/changeEmail', function (req, res) { app.post('/changeEmail', errorCatch(async function (req, res) { let newEmail = req.cookies["newEmail"]; - })); + })); // $ RelatedLocation app.post('/doLoginStuff', errorCatch(async function (req, res) { req.session.user = loginStuff(req); - })); + })); // $ RelatedLocation }) diff --git a/javascript/ql/test/query-tests/Security/CWE-352/csurf_api_example.js b/javascript/ql/test/query-tests/Security/CWE-352/csurf_api_example.js index af5752e73447..edc7ccd1bd74 100644 --- a/javascript/ql/test/query-tests/Security/CWE-352/csurf_api_example.js +++ b/javascript/ql/test/query-tests/Security/CWE-352/csurf_api_example.js @@ -39,10 +39,10 @@ function createApiRouter () { res.send('no csrf to get here') }) - router.post('/getProfile_unsafe', cookieParser(), function (req, res) { // NOT OK - may use cookies + router.post('/getProfile_unsafe', cookieParser(), function (req, res) { // $ Alert - may use cookies let newEmail = req.cookies["newEmail"]; res.send('no csrf to get here') - }) + }) // $ RelatedLocation return router } diff --git a/javascript/ql/test/query-tests/Security/CWE-352/csurf_example.js b/javascript/ql/test/query-tests/Security/CWE-352/csurf_example.js index e464240f7cc9..2ef368b1ce86 100644 --- a/javascript/ql/test/query-tests/Security/CWE-352/csurf_example.js +++ b/javascript/ql/test/query-tests/Security/CWE-352/csurf_example.js @@ -15,7 +15,7 @@ var app = express() // parse cookies // we need this because "cookie" is true in csrfProtection -app.use(cookieParser()) +app.use(cookieParser()) // $ Alert app.get('/form', csrfProtection, function (req, res) { // OK let newEmail = req.cookies["newEmail"]; @@ -28,7 +28,7 @@ app.post('/process', parseForm, csrfProtection, function (req, res) { // OK res.send('data is being processed') }) -app.post('/process_unsafe', parseForm, function (req, res) { // NOT OK +app.post('/process_unsafe', parseForm, function (req, res) { let newEmail = req.cookies["newEmail"]; res.send('data is being processed') -}) +}) // $ RelatedLocation diff --git a/javascript/ql/test/query-tests/Security/CWE-352/fastify.js b/javascript/ql/test/query-tests/Security/CWE-352/fastify.js index 319249c3bbd3..93cad12fbdc0 100644 --- a/javascript/ql/test/query-tests/Security/CWE-352/fastify.js +++ b/javascript/ql/test/query-tests/Security/CWE-352/fastify.js @@ -2,7 +2,7 @@ const fastify = require('fastify') const app = fastify(); -app.register(require('fastify-cookie')); +app.register(require('fastify-cookie')); // $ Alert app.register(require('fastify-csrf')); app.route({ @@ -17,10 +17,10 @@ app.route({ app.route({ method: 'POST', path: '/', - handler: async (req, reply) => { // NOT OK - lacks CSRF protection + handler: async (req, reply) => { // lacks CSRF protection req.session.blah; return req.body - } + } // $ RelatedLocation }) diff --git a/javascript/ql/test/query-tests/Security/CWE-352/fastify2.js b/javascript/ql/test/query-tests/Security/CWE-352/fastify2.js index 1fe557551d2b..9fb5c1358407 100644 --- a/javascript/ql/test/query-tests/Security/CWE-352/fastify2.js +++ b/javascript/ql/test/query-tests/Security/CWE-352/fastify2.js @@ -4,7 +4,7 @@ const fp = require('fastify-plugin'); const app = fastify(); function plugin(app) { - app.register(require('fastify-cookie')); + app.register(require('fastify-cookie')); // $ Alert app.register(require('fastify-csrf')); } app.register(fp(plugin)); @@ -21,10 +21,10 @@ app.route({ app.route({ method: 'POST', path: '/', - handler: async (req, reply) => { // NOT OK - lacks CSRF protection + handler: async (req, reply) => { // lacks CSRF protection req.session.blah; return req.body - } + } // $ RelatedLocation }) diff --git a/javascript/ql/test/query-tests/Security/CWE-352/lusca_example.js b/javascript/ql/test/query-tests/Security/CWE-352/lusca_example.js index ffc6ba474f20..c841eb239662 100644 --- a/javascript/ql/test/query-tests/Security/CWE-352/lusca_example.js +++ b/javascript/ql/test/query-tests/Security/CWE-352/lusca_example.js @@ -6,7 +6,7 @@ var parseForm = bodyParser.urlencoded({ extended: false }) var lusca = require('lusca'); var app = express() -app.use(cookieParser()) +app.use(cookieParser()) // $ Alert app.post('/process', parseForm, lusca.csrf(), function (req, res) { // OK let newEmail = req.cookies["newEmail"]; @@ -23,12 +23,12 @@ app.post('/process', parseForm, lusca({csrf:{}}), function (req, res) { // OK res.send('data is being processed') }) -app.post('/process', parseForm, lusca(), function (req, res) { // NOT OK - missing csrf option +app.post('/process', parseForm, lusca(), function (req, res) { // missing csrf option let newEmail = req.cookies["newEmail"]; res.send('data is being processed') -}) +}) // $ RelatedLocation -app.post('/process_unsafe', parseForm, function (req, res) { // NOT OK +app.post('/process_unsafe', parseForm, function (req, res) { let newEmail = req.cookies["newEmail"]; res.send('data is being processed') -}) +}) // $ RelatedLocation diff --git a/javascript/ql/test/query-tests/Security/CWE-352/tst.js b/javascript/ql/test/query-tests/Security/CWE-352/tst.js index 061dfdba6611..3f688f95d3f8 100644 --- a/javascript/ql/test/query-tests/Security/CWE-352/tst.js +++ b/javascript/ql/test/query-tests/Security/CWE-352/tst.js @@ -3,11 +3,11 @@ const cookieParser = require('cookie-parser') const csrf = require('csurf') const app = express() -app.use(cookieParser()) +app.use(cookieParser()) // $ Alert -app.post('/unsafe', (req, res) => { // NOT OK +app.post('/unsafe', (req, res) => { req.cookies.x; -}); +}); // $ RelatedLocation function middlewares() { return express.Router() diff --git a/javascript/ql/test/query-tests/Security/CWE-352/unused_cookies.js b/javascript/ql/test/query-tests/Security/CWE-352/unused_cookies.js index 133304faac98..9b0a675c4260 100644 --- a/javascript/ql/test/query-tests/Security/CWE-352/unused_cookies.js +++ b/javascript/ql/test/query-tests/Security/CWE-352/unused_cookies.js @@ -3,14 +3,14 @@ let cookieParser = require('cookie-parser'); let app = express(); -app.use(cookieParser()); +app.use(cookieParser()); // $ Alert -app.post('/doSomethingTerrible', (req, res) => { // NOT OK - uses cookies +app.post('/doSomethingTerrible', (req, res) => { // uses cookies if (req.cookies['secret'] === app.secret) { somethingTerrible(); } res.end('Ok'); -}); +}); // $ RelatedLocation app.post('/doSomethingElse', (req, res) => { // OK - doesn't actually use cookies somethingElse(req.query['data']); @@ -26,14 +26,14 @@ app.post('/doWithCaptcha', (req, res) => { // OK - attacker can't guess the capt res.end('Ok'); }); -app.post('/user', (req, res) => { // NOT OK - access to req.user is unprotected +app.post('/user', (req, res) => { // access to req.user is unprotected somethingElse(req.user.name); res.end('Ok'); -}); +}); // $ RelatedLocation -app.post('/session', (req, res) => { // NOT OK - access to req.session is unprotected +app.post('/session', (req, res) => { // access to req.session is unprotected somethingElse(req.session.name); res.end('Ok'); -}); +}); // $ RelatedLocation app.listen(); From cd0fd02e74f0206cc782a594e7b4909e2f41910f Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 19 Feb 2025 14:34:56 +0100 Subject: [PATCH 3/8] Rust: Remove 'Source' annotations from same line as Alert Source tags should no longer be used when on the same line as the Alert. The ones in this file went unnoticed however because *all* of them were on the same line as an Alert, which made the test library ignore all Source tags. --- .../test/query-tests/security/CWE-328/test.rs | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/rust/ql/test/query-tests/security/CWE-328/test.rs b/rust/ql/test/query-tests/security/CWE-328/test.rs index 768ab8a3f431..bb7316b3dce6 100644 --- a/rust/ql/test/query-tests/security/CWE-328/test.rs +++ b/rust/ql/test/query-tests/security/CWE-328/test.rs @@ -11,32 +11,32 @@ fn test_hash_algorithms( // MD5 _ = md5::Md5::digest(harmless); - _ = md5::Md5::digest(credit_card_no); // $ Source Alert[rust/weak-sensitive-data-hashing] - _ = md5::Md5::digest(password); // $ Source Alert[rust/weak-sensitive-data-hashing] + _ = md5::Md5::digest(credit_card_no); // $ Alert[rust/weak-sensitive-data-hashing] + _ = md5::Md5::digest(password); // $ Alert[rust/weak-sensitive-data-hashing] _ = md5::Md5::digest(encrypted_password); // MD5 (alternative / older library) _ = md5_alt::compute(harmless); - _ = md5_alt::compute(credit_card_no); // $ Source Alert[rust/weak-sensitive-data-hashing] - _ = md5_alt::compute(password); // $ Source Alert[rust/weak-sensitive-data-hashing] + _ = md5_alt::compute(credit_card_no); // $ Alert[rust/weak-sensitive-data-hashing] + _ = md5_alt::compute(password); // $ Alert[rust/weak-sensitive-data-hashing] _ = md5_alt::compute(encrypted_password); // SHA-1 _ = sha1::Sha1::digest(harmless); - _ = sha1::Sha1::digest(credit_card_no); // $ Source Alert[rust/weak-sensitive-data-hashing] - _ = sha1::Sha1::digest(password); // $ Source Alert[rust/weak-sensitive-data-hashing] + _ = sha1::Sha1::digest(credit_card_no); // $ Alert[rust/weak-sensitive-data-hashing] + _ = sha1::Sha1::digest(password); // $ Alert[rust/weak-sensitive-data-hashing] _ = sha1::Sha1::digest(encrypted_password); // SHA-1 checked _ = sha1_checked::Sha1::digest(harmless); - _ = sha1_checked::Sha1::digest(credit_card_no); // $ Source Alert[rust/weak-sensitive-data-hashing] - _ = sha1_checked::Sha1::digest(password); // $ Source Alert[rust/weak-sensitive-data-hashing] + _ = sha1_checked::Sha1::digest(credit_card_no); // $ Alert[rust/weak-sensitive-data-hashing] + _ = sha1_checked::Sha1::digest(password); // $ Alert[rust/weak-sensitive-data-hashing] _ = sha1_checked::Sha1::digest(encrypted_password); // SHA-256 (appropriate for sensitive data hashing) _ = sha3::Sha3_256::digest(harmless); _ = sha3::Sha3_256::digest(credit_card_no); - _ = sha3::Sha3_256::digest(password); // $ Source Alert[rust/weak-sensitive-data-hashing] + _ = sha3::Sha3_256::digest(password); // $ Alert[rust/weak-sensitive-data-hashing] _ = sha3::Sha3_256::digest(encrypted_password); // Argon2 (appropriate for password hashing) @@ -57,11 +57,11 @@ fn test_hash_code_patterns( // hash different types of data _ = md5::Md5::digest(harmless_str); - _ = md5::Md5::digest(password_str); // $ Source Alert[rust/weak-sensitive-data-hashing] + _ = md5::Md5::digest(password_str); // $ Alert[rust/weak-sensitive-data-hashing] _ = md5::Md5::digest(harmless_arr); - _ = md5::Md5::digest(password_arr); // $ Source Alert[rust/weak-sensitive-data-hashing] + _ = md5::Md5::digest(password_arr); // $ Alert[rust/weak-sensitive-data-hashing] _ = md5::Md5::digest(harmless_vec); - _ = md5::Md5::digest(password_vec); // $ Source Alert[rust/weak-sensitive-data-hashing] + _ = md5::Md5::digest(password_vec); // $ Alert[rust/weak-sensitive-data-hashing] // hash through a hasher object let mut md5_hasher = md5::Md5::new(); @@ -74,7 +74,7 @@ fn test_hash_code_patterns( _ = md5::Md5::new().chain_update(harmless).chain_update(password).chain_update(harmless).finalize(); // $ MISSING: Alert[rust/weak-sensitive-data-hashing] _ = md5::Md5::new_with_prefix(harmless).finalize(); - _ = md5::Md5::new_with_prefix(password).finalize(); // $ Source Alert[rust/weak-sensitive-data-hashing] + _ = md5::Md5::new_with_prefix(password).finalize(); // $ Alert[rust/weak-sensitive-data-hashing] // hash transformed data _ = md5::Md5::digest(harmless.trim()); From 694f01ab7861c3dfdb10e7ff16853544cddedc25 Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 25 Feb 2025 11:57:01 +0100 Subject: [PATCH 4/8] Fix column count and add clarifying comment --- shared/util/codeql/util/test/InlineExpectationsTest.qll | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/shared/util/codeql/util/test/InlineExpectationsTest.qll b/shared/util/codeql/util/test/InlineExpectationsTest.qll index baa3b2d44404..1947f8ef88db 100644 --- a/shared/util/codeql/util/test/InlineExpectationsTest.qll +++ b/shared/util/codeql/util/test/InlineExpectationsTest.qll @@ -790,8 +790,11 @@ module TestPostProcessing { queryResults(mainResultSet(), row, column + 1, element) | getQueryKind() = "path-problem" and - n = exactDivide(column - 8, 3) + // Skip over `alert, source, sink, message`, counting entities as two columns (7 columns in total). + // Then pick the first column from each related location, which each is an `entity, message` pair (3 columns). + n = exactDivide(column - 7, 3) or + // Like above, but only skip over `alert, message` initially (3 columns in total). getQueryKind() = "problem" and n = exactDivide(column - 3, 3) ) From ae161f16548d16f49b77862281119b0b46c4c63e Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 25 Feb 2025 11:58:54 +0100 Subject: [PATCH 5/8] Add meta-tests for inline expectation with related locations --- .../ql/test/utils/inline-tests/InlineTests.cs | 22 ++++++++++++++++++- .../PathProblemQueryRelatedLocs.expected | 14 ++++++++++++ .../PathProblemQueryRelatedLocs.qlref | 2 ++ .../ProblemQueryRelatedLocs.qlref | 2 ++ .../PathProblemQueryRelatedLocs.expected | 2 ++ .../queries/PathProblemQueryRelatedLocs.ql | 22 +++++++++++++++++++ .../ProblemQueryWithRelatedLocs.expected | 0 .../queries/ProblemQueryWithRelatedLocs.ql | 12 ++++++++++ 8 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 csharp/ql/test/utils/inline-tests/PathProblemQueryRelatedLocs.expected create mode 100644 csharp/ql/test/utils/inline-tests/PathProblemQueryRelatedLocs.qlref create mode 100644 csharp/ql/test/utils/inline-tests/ProblemQueryRelatedLocs.qlref create mode 100644 csharp/ql/test/utils/inline-tests/queries/PathProblemQueryRelatedLocs.expected create mode 100644 csharp/ql/test/utils/inline-tests/queries/PathProblemQueryRelatedLocs.ql create mode 100644 csharp/ql/test/utils/inline-tests/queries/ProblemQueryWithRelatedLocs.expected create mode 100644 csharp/ql/test/utils/inline-tests/queries/ProblemQueryWithRelatedLocs.ql diff --git a/csharp/ql/test/utils/inline-tests/InlineTests.cs b/csharp/ql/test/utils/inline-tests/InlineTests.cs index c30823425ca7..0ee2b019b49d 100644 --- a/csharp/ql/test/utils/inline-tests/InlineTests.cs +++ b/csharp/ql/test/utils/inline-tests/InlineTests.cs @@ -16,6 +16,14 @@ void Problems() // correct expectation comment x = "Alert"; // $ Alert[problem-query] + + // correct expectation comments with a related location + var related = "Related"; // $ RelatedLocation[problem-query-with-related-loc] + x = "Alert:1"; // $ Alert[problem-query-with-related-loc] + + // expectation comments missing the related location + related = "Related"; + x = "Alert:1"; // $ Alert[problem-query-with-related-loc] } void PathProblems() @@ -78,5 +86,17 @@ void PathProblems() // 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 + + // correct expectation comments with a related location + source = "Source"; // $ Source[path-problem-query-with-related-loc] + sink = "Sink"; // $ Sink[path-problem-query-with-related-loc] + var related = "Related"; // $ RelatedLocation[path-problem-query-with-related-loc] + x = "Alert:3:2:1"; // $ Alert[path-problem-query-with-related-loc] + + // expectation comments missing the related location + source = "Source"; // $ Source[path-problem-query-with-related-loc] + sink = "Sink"; // $ Sink[path-problem-query-with-related-loc] + related = "Related"; + x = "Alert:3:2:1"; // $ Alert[path-problem-query-with-related-loc] } -} \ No newline at end of file +} diff --git a/csharp/ql/test/utils/inline-tests/PathProblemQueryRelatedLocs.expected b/csharp/ql/test/utils/inline-tests/PathProblemQueryRelatedLocs.expected new file mode 100644 index 000000000000..a240cc62228f --- /dev/null +++ b/csharp/ql/test/utils/inline-tests/PathProblemQueryRelatedLocs.expected @@ -0,0 +1,14 @@ +#select +| InlineTests.cs:94:13:94:25 | "Alert:3:2:1" | InlineTests.cs:91:18:91:25 | "Source" | InlineTests.cs:92:16:92:21 | "Sink" | This is a problem with $@ | InlineTests.cs:93:23:93:31 | "Related" | a related location | +| InlineTests.cs:100:13:100:25 | "Alert:3:2:1" | InlineTests.cs:97:18:97:25 | "Source" | InlineTests.cs:98:16:98:21 | "Sink" | This is a problem with $@ | InlineTests.cs:99:19:99:27 | "Related" | a related location | +edges +testFailures +| InlineTests.cs:6:26:6:35 | // ... | Missing result: Alert | +| InlineTests.cs:12:34:12:43 | // ... | Missing result: Alert | +| InlineTests.cs:32:32:32:42 | // ... | Missing result: Source | +| InlineTests.cs:33:28:33:36 | // ... | Missing result: Sink | +| InlineTests.cs:34:30:34:39 | // ... | Missing result: Alert | +| InlineTests.cs:37:28:37:38 | // ... | Missing result: Source | +| InlineTests.cs:38:24:38:32 | // ... | Missing result: Sink | +| InlineTests.cs:39:33:39:42 | // ... | Missing result: Alert | +| InlineTests.cs:93:34:93:90 | // ... | Missing result: RelatedLocation[path-problem-query-with-related-loc] | diff --git a/csharp/ql/test/utils/inline-tests/PathProblemQueryRelatedLocs.qlref b/csharp/ql/test/utils/inline-tests/PathProblemQueryRelatedLocs.qlref new file mode 100644 index 000000000000..06932a47f7a2 --- /dev/null +++ b/csharp/ql/test/utils/inline-tests/PathProblemQueryRelatedLocs.qlref @@ -0,0 +1,2 @@ +query: utils/inline-tests/queries/PathProblemQueryRelatedLocs.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/csharp/ql/test/utils/inline-tests/ProblemQueryRelatedLocs.qlref b/csharp/ql/test/utils/inline-tests/ProblemQueryRelatedLocs.qlref new file mode 100644 index 000000000000..13444112b6a6 --- /dev/null +++ b/csharp/ql/test/utils/inline-tests/ProblemQueryRelatedLocs.qlref @@ -0,0 +1,2 @@ +query: utils/inline-tests/queries/ProblemQueryRelatedLocs.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/csharp/ql/test/utils/inline-tests/queries/PathProblemQueryRelatedLocs.expected b/csharp/ql/test/utils/inline-tests/queries/PathProblemQueryRelatedLocs.expected new file mode 100644 index 000000000000..5c153698e9e8 --- /dev/null +++ b/csharp/ql/test/utils/inline-tests/queries/PathProblemQueryRelatedLocs.expected @@ -0,0 +1,2 @@ +edges +#select diff --git a/csharp/ql/test/utils/inline-tests/queries/PathProblemQueryRelatedLocs.ql b/csharp/ql/test/utils/inline-tests/queries/PathProblemQueryRelatedLocs.ql new file mode 100644 index 000000000000..e13c3accfe19 --- /dev/null +++ b/csharp/ql/test/utils/inline-tests/queries/PathProblemQueryRelatedLocs.ql @@ -0,0 +1,22 @@ +/** + * @kind path-problem + * @id path-problem-query-with-related-loc + */ + +import csharp + +query predicate edges(StringLiteral sl1, StringLiteral sl2) { none() } + +from StringLiteral alert, StringLiteral source, StringLiteral sink, StringLiteral related +where + exists(string regexp, int sourceOffset, int sinkOffset, int relatedOffset | + regexp = "Alert:([0-9]+):([0-9]+):([0-9]+)" + | + sourceOffset = alert.getValue().regexpCapture(regexp, 1).toInt() and + sinkOffset = alert.getValue().regexpCapture(regexp, 2).toInt() and + relatedOffset = alert.getValue().regexpCapture(regexp, 3).toInt() and + source.getLocation().getStartLine() = alert.getLocation().getStartLine() - sourceOffset and + sink.getLocation().getStartLine() = alert.getLocation().getStartLine() - sinkOffset and + related.getLocation().getStartLine() = alert.getLocation().getStartLine() - relatedOffset + ) +select alert, source, sink, "This is a problem with $@", related, "a related location" diff --git a/csharp/ql/test/utils/inline-tests/queries/ProblemQueryWithRelatedLocs.expected b/csharp/ql/test/utils/inline-tests/queries/ProblemQueryWithRelatedLocs.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/csharp/ql/test/utils/inline-tests/queries/ProblemQueryWithRelatedLocs.ql b/csharp/ql/test/utils/inline-tests/queries/ProblemQueryWithRelatedLocs.ql new file mode 100644 index 000000000000..5fb28cdd5821 --- /dev/null +++ b/csharp/ql/test/utils/inline-tests/queries/ProblemQueryWithRelatedLocs.ql @@ -0,0 +1,12 @@ +/** + * @kind problem + * @id problem-query-with-related-locs + */ + +import csharp + +from StringLiteral sl, StringLiteral related, int offset +where + sl.getValue().regexpCapture("Alert:([0-9]+)", 1).toInt() = offset and + related.getLocation().getStartLine() = sl.getLocation().getStartLine() - offset +select sl, "This is a problem with $@", related, "a related location" From 92e1023d001af1cf8c0aeb884e91ec222c00fb28 Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 25 Feb 2025 11:59:11 +0100 Subject: [PATCH 6/8] Update line numbers due to addition of new test code --- .../inline-tests/PathProblemQuery.expected | 52 +++++++++---------- .../utils/inline-tests/ProblemQuery.expected | 4 +- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/csharp/ql/test/utils/inline-tests/PathProblemQuery.expected b/csharp/ql/test/utils/inline-tests/PathProblemQuery.expected index 76674dc15695..39e82ef30588 100644 --- a/csharp/ql/test/utils/inline-tests/PathProblemQuery.expected +++ b/csharp/ql/test/utils/inline-tests/PathProblemQuery.expected @@ -1,31 +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 | +| InlineTests.cs:34:17:34:27 | "Alert:2:1" | InlineTests.cs:32:22:32:29 | "Source" | InlineTests.cs:33:20:33:25 | "Sink" | This is a problem | +| InlineTests.cs:44:13:44:23 | "Alert:2:1" | InlineTests.cs:42:18:42:25 | "Source" | InlineTests.cs:43:16:43:21 | "Sink" | This is a problem | +| InlineTests.cs:49:13:49:23 | "Alert:2:1" | InlineTests.cs:47:18:47:25 | "Source" | InlineTests.cs:48:16:48:21 | "Sink" | This is a problem | +| InlineTests.cs:53:13:53:23 | "Alert:1:0" | InlineTests.cs:52:18:52:25 | "Source" | InlineTests.cs:53:13:53:23 | "Alert:1:0" | This is a problem | +| InlineTests.cs:57:13:57:23 | "Alert:0:1" | InlineTests.cs:57:13:57:23 | "Alert:0:1" | InlineTests.cs:56:16:56:21 | "Sink" | This is a problem | +| InlineTests.cs:62:13:62:23 | "Alert:2:1" | InlineTests.cs:60:18:60:25 | "Source" | InlineTests.cs:61:16:61:21 | "Sink" | This is a problem | +| InlineTests.cs:67:13:67:23 | "Alert:2:1" | InlineTests.cs:65:18:65:25 | "Source" | InlineTests.cs:66:16:66:21 | "Sink" | This is a problem | +| InlineTests.cs:72:13:72:23 | "Alert:2:1" | InlineTests.cs:70:18:70:25 | "Source" | InlineTests.cs:71:16:71:21 | "Sink" | This is a problem | +| InlineTests.cs:76:13:76:23 | "Alert:1:0" | InlineTests.cs:75:18:75:25 | "Source" | InlineTests.cs:76:13:76:23 | "Alert:1:0" | This is a problem | +| InlineTests.cs:80:13:80:23 | "Alert:1:0" | InlineTests.cs:79:18:79:25 | "Source" | InlineTests.cs:80:13:80:23 | "Alert:1:0" | This is a problem | +| InlineTests.cs:84:13:84:23 | "Alert:0:1" | InlineTests.cs:84:13:84:23 | "Alert:0:1" | InlineTests.cs:83:16:83:21 | "Sink" | This is a problem | +| InlineTests.cs:88:13:88:23 | "Alert:0:1" | InlineTests.cs:88:13:88:23 | "Alert:0:1" | InlineTests.cs:87:16:87: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 | +| InlineTests.cs:37:28:37:38 | // ... | Missing result: Source | +| InlineTests.cs:38:24:38:32 | // ... | Missing result: Sink | +| InlineTests.cs:39:33:39:42 | // ... | Missing result: Alert | +| InlineTests.cs:42:18:42:25 | "Source" | Unexpected result: Source | +| InlineTests.cs:43:16:43:21 | "Sink" | Unexpected result: Sink | +| InlineTests.cs:44:13:44:23 | InlineTests.cs:42:18:42:25 | Unexpected result: Alert | +| InlineTests.cs:66:16:66:21 | "Sink" | Unexpected result: Sink=source2 | +| InlineTests.cs:66:24:66:60 | // ... | Missing result: Sink[path-problem-query]=source1 | +| InlineTests.cs:72:13:72:23 | InlineTests.cs:70:18:70:25 | Unexpected result: Alert=source3 | +| InlineTests.cs:72:26:72:63 | // ... | Missing result: Alert[path-problem-query]=source2 | +| InlineTests.cs:80:13:80:23 | "Alert:1:0" | Unexpected result: Alert=source5 | +| InlineTests.cs:80:26:80:63 | // ... | Missing result: Alert[path-problem-query]=source4 | +| InlineTests.cs:87:16:87:21 | "Sink" | Unexpected result: Sink=sink1 | +| InlineTests.cs:87:24:87:58 | // ... | Missing result: Sink[path-problem-query]=sink2 | diff --git a/csharp/ql/test/utils/inline-tests/ProblemQuery.expected b/csharp/ql/test/utils/inline-tests/ProblemQuery.expected index 88fe5019fa2a..09a7b4bafd46 100644 --- a/csharp/ql/test/utils/inline-tests/ProblemQuery.expected +++ b/csharp/ql/test/utils/inline-tests/ProblemQuery.expected @@ -5,5 +5,5 @@ 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 | +| InlineTests.cs:34:30:34:39 | // ... | Missing result: Alert | +| InlineTests.cs:39:33:39:42 | // ... | Missing result: Alert | From 5f2e5ab8c390fab682ebae091d79c3dc15a9e3b2 Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 25 Feb 2025 14:52:19 +0100 Subject: [PATCH 7/8] Rename file and ID to match .qlref and other query --- ...ithRelatedLocs.expected => ProblemQueryRelatedLocs.expected} | 0 ...roblemQueryWithRelatedLocs.ql => ProblemQueryRelatedLocs.ql} | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename csharp/ql/test/utils/inline-tests/queries/{ProblemQueryWithRelatedLocs.expected => ProblemQueryRelatedLocs.expected} (100%) rename csharp/ql/test/utils/inline-tests/queries/{ProblemQueryWithRelatedLocs.ql => ProblemQueryRelatedLocs.ql} (89%) diff --git a/csharp/ql/test/utils/inline-tests/queries/ProblemQueryWithRelatedLocs.expected b/csharp/ql/test/utils/inline-tests/queries/ProblemQueryRelatedLocs.expected similarity index 100% rename from csharp/ql/test/utils/inline-tests/queries/ProblemQueryWithRelatedLocs.expected rename to csharp/ql/test/utils/inline-tests/queries/ProblemQueryRelatedLocs.expected diff --git a/csharp/ql/test/utils/inline-tests/queries/ProblemQueryWithRelatedLocs.ql b/csharp/ql/test/utils/inline-tests/queries/ProblemQueryRelatedLocs.ql similarity index 89% rename from csharp/ql/test/utils/inline-tests/queries/ProblemQueryWithRelatedLocs.ql rename to csharp/ql/test/utils/inline-tests/queries/ProblemQueryRelatedLocs.ql index 5fb28cdd5821..c6b584d95074 100644 --- a/csharp/ql/test/utils/inline-tests/queries/ProblemQueryWithRelatedLocs.ql +++ b/csharp/ql/test/utils/inline-tests/queries/ProblemQueryRelatedLocs.ql @@ -1,6 +1,6 @@ /** * @kind problem - * @id problem-query-with-related-locs + * @id problem-query-with-related-loc */ import csharp From bb8f4529bf8e2fd5671b9c118351aec1717a6b1c Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 25 Feb 2025 14:52:32 +0100 Subject: [PATCH 8/8] Fix bug when RelatedLocation was used with a query ID --- .../inline-tests/PathProblemQueryRelatedLocs.expected | 2 +- .../utils/inline-tests/ProblemQueryRelatedLocs.expected | 9 +++++++++ shared/util/codeql/util/test/InlineExpectationsTest.qll | 9 ++++++++- 3 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 csharp/ql/test/utils/inline-tests/ProblemQueryRelatedLocs.expected diff --git a/csharp/ql/test/utils/inline-tests/PathProblemQueryRelatedLocs.expected b/csharp/ql/test/utils/inline-tests/PathProblemQueryRelatedLocs.expected index a240cc62228f..a902691b839b 100644 --- a/csharp/ql/test/utils/inline-tests/PathProblemQueryRelatedLocs.expected +++ b/csharp/ql/test/utils/inline-tests/PathProblemQueryRelatedLocs.expected @@ -11,4 +11,4 @@ testFailures | InlineTests.cs:37:28:37:38 | // ... | Missing result: Source | | InlineTests.cs:38:24:38:32 | // ... | Missing result: Sink | | InlineTests.cs:39:33:39:42 | // ... | Missing result: Alert | -| InlineTests.cs:93:34:93:90 | // ... | Missing result: RelatedLocation[path-problem-query-with-related-loc] | +| InlineTests.cs:99:19:99:27 | "Related" | Unexpected result: RelatedLocation | diff --git a/csharp/ql/test/utils/inline-tests/ProblemQueryRelatedLocs.expected b/csharp/ql/test/utils/inline-tests/ProblemQueryRelatedLocs.expected new file mode 100644 index 000000000000..757bf8fb44ce --- /dev/null +++ b/csharp/ql/test/utils/inline-tests/ProblemQueryRelatedLocs.expected @@ -0,0 +1,9 @@ +#select +| InlineTests.cs:22:13:22:21 | "Alert:1" | This is a problem with $@ | InlineTests.cs:21:23:21:31 | "Related" | a related location | +| InlineTests.cs:26:13:26:21 | "Alert:1" | This is a problem with $@ | InlineTests.cs:25:19:25:27 | "Related" | a related location | +testFailures +| InlineTests.cs:6:26:6:35 | // ... | Missing result: Alert | +| InlineTests.cs:12:34:12:43 | // ... | Missing result: Alert | +| InlineTests.cs:25:19:25:27 | "Related" | Unexpected result: RelatedLocation | +| InlineTests.cs:34:30:34:39 | // ... | Missing result: Alert | +| InlineTests.cs:39:33:39:42 | // ... | Missing result: Alert | diff --git a/shared/util/codeql/util/test/InlineExpectationsTest.qll b/shared/util/codeql/util/test/InlineExpectationsTest.qll index 1947f8ef88db..a618756699a2 100644 --- a/shared/util/codeql/util/test/InlineExpectationsTest.qll +++ b/shared/util/codeql/util/test/InlineExpectationsTest.qll @@ -892,12 +892,19 @@ module TestPostProcessing { not hasPathProblemSink(row, location, _, _) } + private predicate shouldReportRelatedLocations() { + exists(string tag | + hasExpectationWithValue(tag, _) and + PathProblemSourceTestInput::tagMatches(tag, "RelatedLocation") + ) + } + private predicate hasRelatedLocation( int row, TestLocation location, string element, string tag ) { getQueryKind() = ["problem", "path-problem"] and location = getRelatedLocation(row, _, element) and - hasExpectationWithValue("RelatedLocation", _) and + shouldReportRelatedLocations() and tag = "RelatedLocation" and not hasAlert(row, location, _, _) and not hasPathProblemSource(row, location, _, _, _) and