JS: Update OK-style comments to $-style

javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/react.js

@@ -28,21 +28,21 @@ import { useRouter } from 'next/router'
 
 export function nextRouter() {
   const router = useRouter();
-  return <span onClick={() => router.push(document.location.hash.substr(1))}>Click to XSS 1</span> // NOT OK
+  return <span onClick={() => router.push(document.location.hash.substr(1))}>Click to XSS 1</span> // $ Alert
 }
 
 import { withRouter } from 'next/router'
 
 function Page({ router }) {
-  return <span onClick={() => router.push(document.location.hash.substr(1))}>Click to XSS 2</span> // NOT OK
+  return <span onClick={() => router.push(document.location.hash.substr(1))}>Click to XSS 2</span> // $ Alert
 }
 
 export const pageWithRouter = withRouter(Page);
 
 export function plainLink() {
-  return <a href={document.location.hash.substr(1)}>my plain link!</a>; // NOT OK
+  return <a href={document.location.hash.substr(1)}>my plain link!</a>; // $ Alert
 }
 
 export function someUnknown() {
-  return <FOO data={document.location.hash.substr(1)}>is safe.</FOO>; // OK
+  return <FOO data={document.location.hash.substr(1)}>is safe.</FOO>;
 }

javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/regexp-exec.js

@@ -2,40 +2,40 @@ import 'dummy';
 
 function extractFromHash() {
     const [, group1] = /#(.*)/.exec(window.location.href);
-    window.location.href = group1; // NOT OK
+    window.location.href = group1; // $ Alert
 }
 
 function extractFromQuery() {
     const [, group1] = /\?(.*)/.exec(window.location.href);
-    window.location.href = group1; // NOT OK
+    window.location.href = group1; // $ Alert
 }
 
 function extractFromProtocol() {
     const [, group1] = /^([a-z]+:)/.exec(window.location.href);
-    window.location.href = group1; // OK
+    window.location.href = group1;
 }
 
 function extractTooMuch() {
     const [, group1] = /(.*)/.exec(window.location.href);
-    window.location.href = group1; // OK
+    window.location.href = group1;
 }
 
 function extractNothing() {
     const [, group1] = /blah#baz/.exec(window.location.href);
-    window.location.href = group1; // OK
+    window.location.href = group1;
 }
 
 function extractWithMatch() {
     const [, group1] = window.location.href.match(/#(.*)/);
-    window.location.href = group1; // NOT OK
+    window.location.href = group1; // $ Alert
 }
 
 function extractWithMatchAll() {
     const [, group1] = window.location.href.matchAll(/#(.*)/)[0];
-    window.location.href = group1; // NOT OK
+    window.location.href = group1; // $ Alert
 }
 
 function extractFromUnknownRegExp() {
     const [, group1] = new RegExp(unknown()).exec(window.location.href);
-    window.location.href = group1; // NOT OK
+    window.location.href = group1; // $ Alert
 }

javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/sanitizer.js

@@ -1,40 +1,40 @@
 function f() {
     let url = window.name;
     if (url.startsWith('https://example.com')) {
-        window.location = url; // NOT OK - can be example.com.evil.com
+        window.location = url; // $ Alert - can be example.com.evil.com
     }
     if (url.startsWith('https://example.com/')) {
-        window.location = url; // OK
+        window.location = url;
     }
     if (url.startsWith('https://example.com//')) {
-        window.location = url; // OK
+        window.location = url;
     }
     if (url.startsWith('https://example.com/foo')) {
-        window.location = url; // OK 
+        window.location = url;
     }
     if (url.startsWith('https://')) {
-        window.location = url; // NOT OK - does not restrict hostname
+        window.location = url; // $ Alert - does not restrict hostname
     }
     if (url.startsWith('https:/')) {
-        window.location = url; // NOT OK - does not restrict hostname
+        window.location = url; // $ Alert - does not restrict hostname
     }
     if (url.startsWith('https:')) {
-        window.location = url; // NOT OK - does not restrict hostname
+        window.location = url; // $ Alert - does not restrict hostname
     }
     if (url.startsWith('/')) {
-        window.location = url; // NOT OK - can be //evil.com
+        window.location = url; // $ Alert - can be //evil.com
     }
     if (url.startsWith('//')) {
-        window.location = url; // NOT OK - can be //evil.com
+        window.location = url; // $ Alert - can be //evil.com
     }
     if (url.startsWith('//example.com')) {
-        window.location = url; // NOT OK - can be //example.com.evil.com
+        window.location = url; // $ Alert - can be //example.com.evil.com
     }
     if (url.startsWith('//example.com/')) {
-        window.location = url; // OK
+        window.location = url;
     }
     if (url.endsWith('https://example.com/')) {
-        window.location = url; // NOT OK - could be evil.com?x=https://example.com/
+        window.location = url; // $ Alert - could be evil.com?x=https://example.com/
     }
     let basedir = whatever() ? 'foo' : 'bar';
     if (url.startsWith('https://example.com/' + basedir)) {

javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/tst.js

@@ -1,21 +1,18 @@
-// NOT OK
-window.location = /.*redirect=([^&]*).*/.exec(document.location.href)[1];
+window.location = /.*redirect=([^&]*).*/.exec(document.location.href)[1]; // $ Alert
 
 (function(){
 	var indirect = /.*redirect=([^&]*).*/;
 	window.location = indirect.exec(document.location.href)[1];
 });
 
-// NOT OK
-window.location = new RegExp('.*redirect=([^&]*).*').exec(document.location.href)[1];
+window.location = new RegExp('.*redirect=([^&]*).*').exec(document.location.href)[1]; // $ Alert
 
 (function(){
 	var indirect = new RegExp('.*redirect=([^&]*).*')
 	window.location = indirect.exec(document.location.href)[1];
 });
 
-// NOT OK
-window.location = new RegExp(/.*redirect=([^&]*).*/).exec(document.location.href)[1];
+window.location = new RegExp(/.*redirect=([^&]*).*/).exec(document.location.href)[1]; // $ Alert
 
 (function(){
 	var indirect = new RegExp(/.*redirect=([^&]*).*/)
@@ -23,7 +20,7 @@ window.location = new RegExp(/.*redirect=([^&]*).*/).exec(document.location.href
 });
 
 function foo(win) {
-	win.location.assign(new RegExp(/.*redirect=([^&]*).*/).exec(win.location.href)[1]); // NOT OK
+	win.location.assign(new RegExp(/.*redirect=([^&]*).*/).exec(win.location.href)[1]); // $ Alert
 }
 
 foo(window);

javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/tst10.js

@@ -1,14 +1,10 @@
 // OK - cannot affect hostname
 location.href = '/foo' + document.location.search.substring(1);
 
-// NOT OK
-location.href = '/' + document.location.search.substring(1);
+location.href = '/' + document.location.search.substring(1); // $ Alert
 
-// NOT OK
-location.href = '//' + document.location.search.substring(1);
+location.href = '//' + document.location.search.substring(1); // $ Alert
 
-// NOT OK
-location.href = '//foo' + document.location.search.substring(1);
+location.href = '//foo' + document.location.search.substring(1); // $ Alert
 
-// NOT OK
-location.href = 'https://foo' + document.location.search.substring(1);
+location.href = 'https://foo' + document.location.search.substring(1); // $ Alert

javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/tst11.js

@@ -1,4 +1,4 @@
-// OK
+
 function foo() {
     var urlParts = document.location.href.split('?');
     var loc = urlParts[0] + "?" + boxes.value;

javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/tst13.js

@@ -1,56 +1,56 @@
 function foo() {
     var payload = document.location.search.substr(1);
     var el = document.createElement("a");
-    el.href = payload; // NOT OK
+    el.href = payload; // $ Alert
     document.body.appendChild(el);
 
     var el = document.createElement("button");
-    el.formaction = payload; // NOT OK
+    el.formaction = payload; // $ Alert
     document.body.appendChild(el);
 
     var el = document.createElement("embed");
-    el.src = payload; // NOT OK
+    el.src = payload; // $ Alert
     document.body.appendChild(el);
 
     var el = document.createElement("form");
-    el.action = payload; // NOT OK
+    el.action = payload; // $ Alert
     document.body.appendChild(el);
 
     var el = document.createElement("frame");
-    el.src = payload; // NOT OK
+    el.src = payload; // $ Alert
     document.body.appendChild(el);
 
     var el = document.createElement("iframe");
-    el.src = payload; // NOT OK
+    el.src = payload; // $ Alert
     document.body.appendChild(el);
 
     var el = document.createElement("input");
-    el.formaction = payload; // NOT OK
+    el.formaction = payload; // $ Alert
     document.body.appendChild(el);
 
     var el = document.createElement("isindex");
-    el.action = payload; // NOT OK
+    el.action = payload; // $ Alert
     document.body.appendChild(el);
 
     var el = document.createElement("isindex");
-    el.formaction = payload; // NOT OK
+    el.formaction = payload; // $ Alert
     document.body.appendChild(el);
 
     var el = document.createElement("object");
-    el.data = payload; // NOT OK
+    el.data = payload; // $ Alert
     document.body.appendChild(el);
 
     var el = document.createElement("script");
-    el.src = payload; // NOT OK
+    el.src = payload; // $ Alert
     document.body.appendChild(el);
 }
 
 (function () {
     self.onmessage = function (e) {
-        importScripts(e); // NOT OK
+        importScripts(e); // $ Alert
     }
     window.onmessage = function (e) {
-        self.importScripts(e); // NOT OK
+        self.importScripts(e); // $ Alert
     }
 })();
 
@@ -58,27 +58,27 @@ function bar() {
     const history = require('history').createBrowserHistory();
     var payload = document.location.search.substr(1);
 
-    history.push(payload); // NOT OK
+    history.push(payload); // $ Alert
 }
 function baz() {
     const history = require('history').createBrowserHistory();
     var payload = history.location.hash.substr(1);
 
-    history.replace(payload); // NOT OK
+    history.replace(payload); // $ Alert
 }
 
 function quz() {
     const history = HistoryLibrary.createBrowserHistory();
     var payload = history.location.hash.substr(1);
 
-    history.replace(payload); // NOT OK
+    history.replace(payload); // $ Alert
 }
 
 function bar() {
     var url = document.location.search.substr(1);
 
-    $("<a>", {href: url}).appendTo("body"); // NOT OK
-    $("#foo").attr("href", url); // NOT OK
-    $("#foo").attr({href: url}); // NOT OK
-    $("<img>", {src: url}).appendTo("body"); // NOT OK
+    $("<a>", {href: url}).appendTo("body"); // $ Alert
+    $("#foo").attr("href", url); // $ Alert
+    $("#foo").attr({href: url}); // $ Alert
+    $("<img>", {src: url}).appendTo("body"); // $ Alert
 }

javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/tst14.js

@@ -2,9 +2,9 @@ function foo() {
     var url = document.location.search.substr(1);
     var el = document.createElement("script");
     el.src = "https://github.com/" + url;
-    document.body.appendChild(el); // OK
+    document.body.appendChild(el);
 
     if (url.startsWith('https://github.com/')) {
-        window.location = url; // OK
+        window.location = url;
     }
 }

javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/tst15.js

@@ -1,22 +1,22 @@
 function foo() {
     var url = document.location.toString();
-    window.location = url.substring(0).substring(1); // OK [INCONSISTENCY] - but not important
-    window.location = url.substring(0, 10).substring(1); // OK [INCONSISTENCY]
-    window.location = url.substring(0, url.indexOf('/', 10)).substring(1); // OK [INCONSISTENCY]
+    window.location = url.substring(0).substring(1); // $ SPURIOUS: Alert - but not important
+    window.location = url.substring(0, 10).substring(1); // $ SPURIOUS: Alert
+    window.location = url.substring(0, url.indexOf('/', 10)).substring(1); // $ SPURIOUS: Alert
 
     var url2 = document.location.toString();
-    window.location = url2.substring(0).substring(unknown()); // NOT OK
-    window.location = url2.substring(0, 10).substring(unknown()); // NOT OK
-    window.location = url2.substring(0, url2.indexOf('/', 10)).substring(unknown()); // NOT OK
+    window.location = url2.substring(0).substring(unknown()); // $ Alert
+    window.location = url2.substring(0, 10).substring(unknown()); // $ Alert
+    window.location = url2.substring(0, url2.indexOf('/', 10)).substring(unknown()); // $ Alert
 
     var search = document.location.search.toString();
-    window.location = search.substring(0).substring(1); // NOT OK
-    window.location = search.substring(0, 10).substring(1); // NOT OK
-    window.location = search.substring(0, search.indexOf('/', 10)).substring(1); // NOT OK
+    window.location = search.substring(0).substring(1); // $ Alert
+    window.location = search.substring(0, 10).substring(1); // $ Alert
+    window.location = search.substring(0, search.indexOf('/', 10)).substring(1); // $ Alert
 }
 
 function bar() {
     var url = new URL(window.location);
-    window.location = url.origin; // OK
-    window.location = url.origin.substring(10); // OK
+    window.location = url.origin;
+    window.location = url.origin.substring(10);
 }

javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/tst2.js

@@ -1,5 +1,4 @@
 function f() {
   var href = window.location.href;
-  // NOT OK
-  window.location = href.substring(href.indexOf('?')+1);
+  window.location = href.substring(href.indexOf('?')+1); // $ Alert
 }

javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/tst3.js

@@ -1,5 +1,5 @@
 function g() {
   var href = window.location.href;
-  // OK
+
   window.location = href.substring(0, href.lastIndexOf('/'));
 }

javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/tst4.js

@@ -1,5 +1,5 @@
 function h() {
   var href = window.location.href;
-  // OK
+
   window.location = "https://github.com?" + href.substring(href.indexOf('?')+1);
 }

javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/tst5.js

@@ -1,2 +1,2 @@
-// OK
+
 window.location = window.location.toString().split('?')[0] + '?' + boxes.value;

javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/tst6.js

@@ -1,9 +1,6 @@
 angular.module('app').directive('foo', function($location) {
   let redirect = $location.search('redirect');
-  // NOT OK
-  window.location = redirect;
-  // NOT OK
-  $location.url(redirect);
-  // NOT OK
-  window.location = $location.search('redirect') + "foo";
+  window.location = redirect; // $ Alert
+  $location.url(redirect); // $ Alert
+  window.location = $location.search('redirect') + "foo"; // $ Alert
 });

javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/tst7.js

@@ -1,5 +1,3 @@
-// NOT OK
-new Worker(document.location.search.substring(1));
+new Worker(document.location.search.substring(1)); // $ Alert
 
-// NOT OK
-$("<script>").attr("src", document.location.search.substring(1));
+$("<script>").attr("src", document.location.search.substring(1)); // $ Alert

javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/tst8.js

@@ -1,6 +1,6 @@
 function k() {
   var search = window.location.search.substr(1);
-  // OK
+
   window.location = "https://github.com" + questionMark() + search;
 }
 

javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/tst9.js

@@ -1,2 +1 @@
-// NOT OK
-document.location = document.location.hash.substring(1)
+document.location = document.location.hash.substring(1) // $ Alert

javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/typed.ts

@@ -5,7 +5,7 @@ export class MyComponent {
         this.doRedirect(params);
     }
     private doRedirect(redirectUri: string) {
-        window.location.replace(redirectUri); // NOT OK
+        window.location.replace(redirectUri); // $ Alert
     }
 }
 
@@ -26,7 +26,7 @@ export class MyTrackingComponent {
     }
 
     private doRedirect(redirectUri: string) {
-        window.location.replace(redirectUri); // NOT OK
+        window.location.replace(redirectUri); // $ Alert
     }
 }
 
@@ -49,10 +49,10 @@ export class WeirdTracking {
     }
 
     private doRedirect(redirectUri: string) {
-        window.location.replace(redirectUri); // NOT OK
+        window.location.replace(redirectUri); // $ Alert
     }
 
     private doRedirect2(redirectUri: string) {
-        window.location.replace(redirectUri); // NOT OK
+        window.location.replace(redirectUri); // $ Alert
     }
 }

javascript/ql/test/query-tests/Security/CWE-601/ServerSideUrlRedirect/ServerSideUrlRedirect.js

@@ -1,6 +1,5 @@
 const app = require("express")();
 
 app.get("/redirect", function (req, res) {
-  // BAD: a request parameter is incorporated without validation into a URL redirect
-  res.redirect(req.query["target"]);
+  res.redirect(req.query["target"]); // $ Alert - a request parameter is incorporated without validation into a URL redirect
 });

javascript/ql/test/query-tests/Security/CWE-601/ServerSideUrlRedirect/ServerSideUrlRedirectGood.js

@@ -3,7 +3,7 @@ const app = require("express")();
 const VALID_REDIRECT = "http://cwe.mitre.org/data/definitions/601.html";
 
 app.get("/redirect", function (req, res) {
-  // GOOD: the request parameter is validated against a known fixed string
+  // OK - the request parameter is validated against a known fixed string
   let target = req.query["target"];
   if (VALID_REDIRECT === target) {
     res.redirect(target);

javascript/ql/test/query-tests/Security/CWE-601/ServerSideUrlRedirect/ServerSideUrlRedirectGood2.js

@@ -12,7 +12,7 @@ function isLocalUrl(path) {
 }
 
 app.get("/redirect", function (req, res) {
-  // GOOD: check that we don't redirect to a different host
+  // OK - check that we don't redirect to a different host
   let target = req.query["target"];
   if (isLocalUrl(target)) {
     res.redirect(target);

javascript/ql/test/query-tests/Security/CWE-601/ServerSideUrlRedirect/express.js

@@ -3,41 +3,36 @@ var express = require('express');
 var app = express();
 
 app.get('/some/path', function(req, res) {
-  // BAD: a request parameter is incorporated without validation into a URL redirect
-  res.redirect(req.param("target"));
+  res.redirect(req.param("target")); // $ Alert - a request parameter is incorporated without validation into a URL redirect
 });
 
 app.all(function(req, res) {
-  // BAD: a request parameter is incorporated without validation into a URL redirect
-  res.header("Location", req.param("target"));
+  res.header("Location", req.param("target")); // $ Alert - a request parameter is incorporated without validation into a URL redirect
 });
 
 app.get('/some/other/path', function(req, res) {
-  // GOOD: request parameter is embedded in query string
+  // OK - request parameter is embedded in query string
   res.redirect(someUrl() + "?target=" + req.param("target"));
 });
 
 const HASH = "#";
 app.get('/some/other/path2', function(req, res) {
-  // GOOD: request parameter is embedded in hash
+  // OK - request parameter is embedded in hash
   res.redirect(someUrl() + (HASH + req.param("anchor")));
 });
 
 app.get('/some/path', function(req, res) {
   var target = req.param("target");
   if (isLocalURL(target))
-    // GOOD: request parameter is sanitized before incorporating it into the redirect
+    // OK - request parameter is sanitized before incorporating it into the redirect
     res.redirect(target);
   else
-    // BAD: sanitization doesn't apply here
-    res.redirect(target);
-  // BAD: sanitization doesn't apply here
-  res.redirect(target);
+    res.redirect(target); // $ Alert - sanitization doesn't apply here
+  res.redirect(target); // $ Alert - sanitization doesn't apply here
 });
 
 app.get('/foo', function(req, res) {
-  // BAD: may be a global redirection
-  res.redirect((req.param('action') && req.param('action') != "") ? req.param('action') : "/google_contacts")
+  res.redirect((req.param('action') && req.param('action') != "") ? req.param('action') : "/google_contacts") // $ Alert - may be a global redirection
 });
 
 app.get('/bar', function(req, res, next) {
@@ -45,7 +40,7 @@ app.get('/bar', function(req, res, next) {
   var url = "/Me/" + handle + "/";
   var qs = querystring.stringify(req.query);
   if (qs.length > 0) url += "?" + qs;
-  // GOOD: local redirect
+  // OK - local redirect
   res.header("Location", url);
 });
 
@@ -57,7 +52,7 @@ router.put('/putasync/:retry/:finalState', function (req, res, next) {
     var pollingUri = 'http://localhost:' + utils.getPort() + '/lro/putasync/' + retry + '/' + finalState.toLowerCase() + '/operationResults/200/';
     var headers = {
       'Azure-AsyncOperation': pollingUri,
-      // GOOD: localhost redirect
+      // OK - localhost redirect
       'Location': pollingUri
     };
     if (retry === 'retry') {
@@ -70,37 +65,34 @@ router.put('/putasync/:retry/:finalState', function (req, res, next) {
 });
 
 app.get('/yet/another/path', function(req, res) {
-  // BAD: a request parameter is incorporated without validation into a URL redirect
-  res.redirect(`${req.param("target")}/foo`);
+  res.redirect(`${req.param("target")}/foo`); // $ Alert - a request parameter is incorporated without validation into a URL redirect
 });
 
 app.get('/hopefully/the/final/path', function(req, res) {
-  // GOOD: request parameter is embedded in query string
+  // OK - request parameter is embedded in query string
   res.redirect(`${someUrl()}?target=${req.param("target")}`);
 });
 
 app.get('/some/path', function(req, res) {
   let target = req.param("target");
 
-  // GOOD: request parameter is checked against whitelist
+  // OK - request parameter is checked against whitelist
   if (SAFE_TARGETS.hasOwnProperty(target))
     res.redirect(target);
   else
-    // BAD: check does not apply here
-    res.redirect(target);
+    res.redirect(target); // $ Alert - check does not apply here
 
-  // GOOD: request parameter is checked against whitelist
+  // OK - request parameter is checked against whitelist
   if (target in SAFE_TARGETS)
     res.redirect(target);
 
-  // BAD: check does not apply here
-  res.redirect(target);
+  res.redirect(target); // $ Alert - check does not apply here
 
-  // GOOD: request parameter is checked against whitelist
+  // OK - request parameter is checked against whitelist
   if (SAFE_TARGETS[target] != undefined)
     res.redirect(target);
 
-  // GOOD: request parameter is checked against whitelist
+  // OK - request parameter is checked against whitelist
   if (void(0) == SAFE_TARGETS[target])
     res.send("Denied!");
   else
@@ -108,19 +100,17 @@ app.get('/some/path', function(req, res) {
 });
 
 app.get('/array/join', function(req, res) {
-  // GOOD: request input embedded in query string
+  // OK - request input embedded in query string
   res.redirect(['index.html?section=', req.query.section].join(''));
 
-  // GOOD: request input still embedded in query string
+  // OK - request input still embedded in query string
   res.redirect(['index.html?section=', '34'].join('') + '&subsection=' + req.query.subsection);
 
-  // BAD: request input becomes before query string
-  res.redirect([req.query.page, '?section=', req.query.section].join(''));
+  res.redirect([req.query.page, '?section=', req.query.section].join('')); // $ Alert - request input becomes before query string
 });
 
 function sendUserToUrl(res, nextUrl) {
-  // BAD: value comes from query parameter
-  res.redrect(nextUrl);
+  res.redrect(nextUrl); // $ Alert - value comes from query parameter
 }
 
 app.get('/call', function(req, res) {
@@ -128,39 +118,39 @@ app.get('/call', function(req, res) {
 });
 
 app.get('/redirect/:user', function(req, res) {
-  res.redirect('/users/' + req.params.user); // GOOD
-  res.redirect('users/' + req.params.user); // GOOD
+  res.redirect('/users/' + req.params.user);
+  res.redirect('users/' + req.params.user);
 
-  res.redirect('/' + req.params.user); // BAD - could go to //evil.com
-  res.redirect('//' + req.params.user); // BAD - could go to //evil.com
-  res.redirect('u' + req.params.user); // BAD - could go to u.evil.com
+  res.redirect('/' + req.params.user); // $ Alert - could go to //evil.com
+  res.redirect('//' + req.params.user); // $ Alert - could go to //evil.com
+  res.redirect('u' + req.params.user); // $ Alert - could go to u.evil.com
 
-  res.redirect('/' + ('/u' + req.params.user)); // BAD - could go to //u.evil.com, but not flagged [INCONSISTENCY]
-  res.redirect('/u' + req.params.user); // GOOD
+  res.redirect('/' + ('/u' + req.params.user)); // $ MISSING: Alert - could go to //u.evil.com, but not flagged
+  res.redirect('/u' + req.params.user);
 });
 
 app.get("foo", (req, res) => {
-  res.redirect(req.query.foo); // NOT OK
+  res.redirect(req.query.foo); // $ Alert
 });
 app.get("bar", ({query}, res) => {
-  res.redirect(query.foo); // NOT OK
+  res.redirect(query.foo); // $ Alert
 })
 
 app.get('/some/path', function(req, res) {
   let target = req.param("target");
   
   if (SAFE_TARGETS.hasOwnProperty(target))
-    res.redirect(target); // OK: request parameter is checked against whitelist
+    res.redirect(target); // OK - request parameter is checked against whitelist
   else
-    res.redirect(target); // NOT OK
+    res.redirect(target); // $ Alert
 
   if (Object.hasOwn(SAFE_TARGETS, target))
-    res.redirect(target); // OK: request parameter is checked against whitelist
+    res.redirect(target); // OK - request parameter is checked against whitelist
   else
-    res.redirect(target); // NOT OK
+    res.redirect(target); // $ Alert
 });
 
 app.get("/foo/:bar/:baz", (req, res) => {
   let myThing = JSON.stringify(req.query).slice(1, -1);
-  res.redirect(myThing); // NOT OK
+  res.redirect(myThing); // $ Alert
 });

javascript/ql/test/query-tests/Security/CWE-601/ServerSideUrlRedirect/koa.js

@@ -4,26 +4,26 @@ const app = new Koa();
 
 app.use(async ctx => {
 	var url = ctx.query.target;
-	ctx.redirect(url); // NOT OK
-	ctx.redirect(`${url}${x}`); // NOT OK
+	ctx.redirect(url); // $ Alert
+	ctx.redirect(`${url}${x}`); // $ Alert
 
 	var isCrossDomainRedirect = urlLib.parse(url || '', false, true).hostname;
 	if(!url || isCrossDomainRedirect) {
-		ctx.redirect('/'); // OK
+		ctx.redirect('/');
 	} else {
-		ctx.redirect(url); // NOT OK
+		ctx.redirect(url); // $ Alert
 	}
 
 	if(!url || isCrossDomainRedirect || url.match(VALID)) {
-		ctx.redirect('/'); // OK
+		ctx.redirect('/');
 	} else {
-		ctx.redirect(url); // possibly OK - flagged anyway
+		ctx.redirect(url); // $ Alert - possibly OK but flagged anyway
 	}
 
 	if(!url || isCrossDomainRedirect || url.match(/[^\w/-]/)) {
-		ctx.redirect('/'); // OK
+		ctx.redirect('/');
 	} else {
-		ctx.redirect(url); // OK
+		ctx.redirect(url);
 	}
 });
 

javascript/ql/test/query-tests/Security/CWE-601/ServerSideUrlRedirect/next.ts

@@ -8,5 +8,5 @@ export async function handler(
   req: NextApiRequest,
   res: NextApiResponse
 ) {
-    res.setHeader("Location", req.body.callbackUrl); // NOT OK
+    res.setHeader("Location", req.body.callbackUrl); // $ Alert
 }

javascript/ql/test/query-tests/Security/CWE-601/ServerSideUrlRedirect/node.js

@@ -3,22 +3,21 @@ var url = require('url');
 
 var server = https.createServer(function(req, res) {
   let target = url.parse(req.url, true).query.target;
-  res.writeHead(302, { Location: target }); // BAD: a request parameter is incorporated without validation into a URL redirect
+  res.writeHead(302, { Location: target }); // $ Alert - a request parameter is incorporated without validation into a URL redirect
 })
 
 server.on('request', (req, res) => {
   let target = url.parse(req.url, true).query.target;
-  // GOOD: local redirects are unproblematic
+  // OK - local redirects are unproblematic
   res.writeHead(302, { Location: '/local/' + target });
-  // BAD: this could be a non-local redirect
-  res.writeHead(302, { Location: '/' + target });
-  // GOOD: localhost redirects are unproblematic
+  res.writeHead(302, { Location: '/' + target }); // $ Alert - this could be a non-local redirect
+  // OK - localhost redirects are unproblematic
   res.writeHead(302, { Location: '//localhost/' + target });
 })
 
 server.on('request', (req, res) => {
   let target = url.parse(req.url, true).query.target;
-  // GOOD: comparison against known URLs
+  // OK - comparison against known URLs
   if (target === 'semmle.com')
     res.writeHead(302, { Location: target });
 })
@@ -27,6 +26,5 @@ server.on('request', (req, res) => {
 server.on('request', (req, res) => {
   let target = url.parse(req.url, true).query.target;
   let me = "me"
-  // BAD: may be a global redirection
-  res.writeHead(302, { Location: target + "?from=" + me });
+  res.writeHead(302, { Location: target + "?from=" + me }); // $ Alert - may be a global redirection
 })

javascript/ql/test/query-tests/Security/CWE-601/ServerSideUrlRedirect/react-native.js

@@ -5,6 +5,6 @@ var app = express();
 
 app.get('/some/path', function(req, res) {
   let tainted = req.param("code");
-  <WebView url={tainted}/>;            // NOT OK
-  <WebView source={{uri: tainted}}/>;  // NOT OK
+  <WebView url={tainted}/>;            // $ Alert
+  <WebView source={{uri: tainted}}/>;  // $ Alert
 });