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/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/PathProblemQueryRelatedLocs.expected b/csharp/ql/test/utils/inline-tests/PathProblemQueryRelatedLocs.expected new file mode 100644 index 000000000000..a902691b839b --- /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:99:19:99:27 | "Related" | Unexpected result: RelatedLocation | 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/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 | 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/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/ProblemQueryRelatedLocs.expected b/csharp/ql/test/utils/inline-tests/queries/ProblemQueryRelatedLocs.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/csharp/ql/test/utils/inline-tests/queries/ProblemQueryRelatedLocs.ql b/csharp/ql/test/utils/inline-tests/queries/ProblemQueryRelatedLocs.ql new file mode 100644 index 000000000000..c6b584d95074 --- /dev/null +++ b/csharp/ql/test/utils/inline-tests/queries/ProblemQueryRelatedLocs.ql @@ -0,0 +1,12 @@ +/** + * @kind problem + * @id problem-query-with-related-loc + */ + +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" 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(); 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()); diff --git a/shared/util/codeql/util/test/InlineExpectationsTest.qll b/shared/util/codeql/util/test/InlineExpectationsTest.qll index 17ac7f1cd257..a618756699a2 100644 --- a/shared/util/codeql/util/test/InlineExpectationsTest.qll +++ b/shared/util/codeql/util/test/InlineExpectationsTest.qll @@ -779,22 +779,36 @@ 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 + // 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) ) } + 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 +892,25 @@ 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 + shouldReportRelatedLocations() 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 +932,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