JS: Move some alerts to their correct location

One of the diffs look confusing but:
Previously parameter {2,3} where flagged, now parameter {1,2} are flagged.

Note that for command injection, the SystemCommandExecution is flagged
despite the test file claiming otherwise.

javascript/ql/test/query-tests/Declarations/UniqueParameterNames/tst.js

@@ -1,7 +1,7 @@
 function f(
-x,
 x, // $ Alert
-\u0078 // $ Alert
+x, // $ Alert
+\u0078
 ) { return; }
 
 this.addPropertyListener(prop.name, function(_, _, _, a) {

javascript/ql/test/query-tests/Electron/NodeIntegration/EnablingNodeIntegration.js

@@ -1,30 +1,30 @@
 const {BrowserWindow} = require('electron')
 
 function test() {
-    var unsafe_1 = { // $ Alert - both enabled
+    var unsafe_1 = { // both enabled
            webPreferences: {
             nodeIntegration: true,
 			nodeIntegrationInWorker: true,
             plugins: true,
             webSecurity: true,
             sandbox: true
-        }
+        } // $ Alert
     };
     
-    var options_1 = { // $ Alert - `nodeIntegrationInWorker` enabled
+    var options_1 = { // `nodeIntegrationInWorker` enabled
 	    webPreferences: {
             plugins: true,
 			nodeIntegrationInWorker: false,
             webSecurity: true,
             sandbox: true     
-        }
+        } // $ Alert
     };
     
-    var pref = { // $ Alert - implicitly enabled
+    var pref = { // implicitly enabled
             plugins: true,
             webSecurity: true,
             sandbox: true     
-        };
+        }; // $ Alert
 	
     var options_2 = {  // $ Alert - implicitly enabled
             webPreferences: pref,

javascript/ql/test/query-tests/React/InconsistentStateUpdate/tst.js

@@ -26,73 +26,73 @@ class C3 extends React.Component {
 class C4 extends React.Component {
   upd8() {
     this.setState({
-      counter: this.state.foo // $ Alert
-    });
+      counter: this.state.foo
+    }); // $ Alert
   }
 }
 
 class C5 extends React.Component {
   upd8() {
     this.setState({
-        foo: { bar: this.state.foo.bar } // $ Alert
-    });
+        foo: { bar: this.state.foo.bar }
+    }); // $ Alert
   }
 }
 
 class C7 extends React.Component {
     upd8a() {
         this.setState({
-            foo: this.state.foo // $ Alert
-        });
+            foo: this.state.foo
+        }); // $ Alert
     }
 
     upd8b() {
         this.setState({
-            foo: this.state.foo // $ Alert
-        });
+            foo: this.state.foo
+        }); // $ Alert
     }
 }
 
 class C8 extends React.Component {
     upd8a() {
         this.setState({
-            foo: this.state.foo + 1 // $ Alert
-        });
+            foo: this.state.foo + 1
+        }); // $ Alert
     }
 
     upd8b() {
         this.setState({
-            foo: this.state.foo + 1 // $ Alert
-        });
+            foo: this.state.foo + 1
+        }); // $ Alert
     }
 }
 
 class C9 extends React.Component {
     upd8a() {
         this.setState({
-            foo: { bar: this.state.foo.bar } // $ Alert
-        });
+            foo: { bar: this.state.foo.bar }
+        }); // $ Alert
     }
 
     upd8b() {
         this.setState({
-            foo: { bar: this.state.foo.bar } // $ Alert
-        });
+            foo: { bar: this.state.foo.bar }
+        }); // $ Alert
     }
 }
 
 class C10 extends React.Component {
     upd8a() {
         this.setState({
-            foo: this.state.foo, // $ Alert
-        });
+            foo: this.state.foo,
             bar: this.state.bar // OK - ignored because it is safe in practice
+        }); // $ Alert
     }
 
     upd8b() {
         this.setState({
-            foo: this.state.foo // $ Alert
-        });
+            foo: this.state.foo
+        }); // $ Alert
     }
 }
 
@@ -100,13 +100,13 @@ class C11 extends React.Component {
     upd8a() {
         var self = this;
         self.setState({
-            foo: self.state.foo // $ Alert
-        });
+            foo: self.state.foo
+        }); // $ Alert
     }
 
     upd8b() {
         this.setState({
-            foo: this.state.foo // $ Alert
-        });
+            foo: this.state.foo
+        }); // $ Alert
     }
 }

javascript/ql/test/query-tests/React/UnusedOrUndefinedStateProperty/undefined.js

@@ -58,10 +58,10 @@ class C5 extends React.Component {
 }
 new C5({writtenInUnknownInitializerObject: 42});
 
-React.createClass({
+React.createClass({ // $ Alert
     render: function() {
         this.state.writtenInKnownInitializerObject;
-        this.state.notWrittenInKnownInitializerObject; // $ Alert
+        this.state.notWrittenInKnownInitializerObject;
         return <div/>;
   },
   getInitialState: function() {

javascript/ql/test/query-tests/Security/CWE-078/CommandInjection/child_process-test.js

@@ -40,18 +40,18 @@ var server = http.createServer(function(req, res) {
 
     let args = [];
     args[0] = "-c";
-    args[1] = cmd;  // $ Alert
-    cp.execFile("/bin/bash", args);
+    args[1] = cmd;
+    cp.execFile("/bin/bash", args); // $ Alert
 
     let args = [];
     args[0] = "-c";
-    args[1] = cmd;  // $ Alert
+    args[1] = cmd;
     run("sh", args);
 
     let args = [];
     args[0] = `-` + "c";
-    args[1] = cmd; // $ Alert
-    cp.execFile(`/bin` + "/bash", args);
+    args[1] = cmd;
+    cp.execFile(`/bin` + "/bash", args); // $ Alert
 
     cp.spawn('cmd.exe', ['/C', 'foo'].concat(["bar", cmd])); // $ Alert
     cp.spawn('cmd.exe', ['/C', 'foo'].concat(cmd)); // $ Alert
@@ -64,7 +64,7 @@ var server = http.createServer(function(req, res) {
 });
 
 function run(cmd, args) {
-  cp.spawn(cmd, args); // OK - the alert happens where `args` is build.
+  cp.spawn(cmd, args); // $ Alert - but note that the sink is where `args` is build.
 }
 
 var util = require("util")
@@ -92,4 +92,4 @@ const router = new Router();
 
 router.get("/ping/:host", async (ctx) => {
   cp.exec("ping " + ctx.params.host); // $ Alert
-});
+});

javascript/ql/test/query-tests/Security/CWE-089/untyped/graphql.js

@@ -6,7 +6,7 @@ const kit = new Octokit();
 
 app.get('/post/:id', function(req, res) {
     const id = req.params.id;
-    const response = kit.graphql(` // $ Alert
+    const response = kit.graphql(`
       query {
         repository(owner: "github", name: "${id}") {
           object(expression: "master:foo") {
@@ -16,7 +16,7 @@ app.get('/post/:id', function(req, res) {
           }
         }
       }
-    `);
+    `); // $ Alert
 });
 
 import { graphql, withCustomRequest } from "@octokit/graphql";
@@ -72,20 +72,20 @@ var root = {
 app.get('/thing/:id', async function(req, res) {
   const id = req.query.id;
   const result = await nativeGraphql(schema, "{ foo" + id + " }", root); // $ Alert
-  
+
   fetch("https://my-grpahql-server.com/graphql", {
     method: "POST",
     headers: {
       "Content-Type": "application/json"
     },
     body: JSON.stringify({
-      query: `{ // $ Alert
+      query: `{
         thing {
           name
           url
           ${id}
         }
-      }`
+      }` // $ Alert
     })
   })
 

javascript/ql/test/query-tests/Security/CWE-1004/tst-httpOnly.js

@@ -11,8 +11,8 @@ app.use(session({
 app.use(session({
     name: 'session',
     keys: ['key1', 'key2'],
-    httpOnly: false // $ Alert
-}))
+    httpOnly: false
+})) // $ Alert
 
 app.use(session({
     name: 'session',
@@ -51,8 +51,8 @@ var flag2 = flag
 app.use(session({
     name: 'session',
     keys: ['key1', 'key2'],
-    httpOnly: flag2 // $ Alert
-}))
+    httpOnly: flag2
+})) // $ Alert
 
 app.get('/a', function (req, res, next) {
     res.cookie('authkey', 'value',
@@ -68,9 +68,9 @@ app.get('/a', function (req, res, next) {
     res.cookie('authkey', 'value',
         {
             maxAge: 9000000000,
-            httpOnly: false, // $ Alert
+            httpOnly: false,
             secure: false
-        });
+        }); // $ Alert
     res.end('ok')
 })
 
@@ -78,8 +78,8 @@ app.get('/a', function (req, res, next) {
     res.cookie('authkey', 'value',
         {
             maxAge: 9000000000
-        });
-    res.end('ok') // $ Alert
+        }); // $ Alert
+    res.end('ok')
 })
 
 app.get('/a', function (req, res, next) {
@@ -95,10 +95,10 @@ app.get('/a', function (req, res, next) {
 app.get('/a', function (req, res, next) {
     let options = {
         maxAge: 9000000000,
-        httpOnly: false, // $ Alert
+        httpOnly: false,
         secure: false
     }
-    res.cookie('authkey', 'value', options);
+    res.cookie('authkey', 'value', options); // $ Alert
     res.end('ok')
 })
 
@@ -301,8 +301,8 @@ app.use(session({
 app.use(session({
     name: 'session',
     keys: ['key1', 'key2'],
-    cookie: { httpOnly: false } // $ Alert
-}))
+    cookie: { httpOnly: false }
+})) // $ Alert
 
 app.use(session({
     name: 'session',
@@ -318,8 +318,8 @@ app.use(session({ // OK - httpOnly is true by default
 app.use(session({
     name: 'mycookie',
     keys: ['key1', 'key2'],
-    cookie: { httpOnly: false } // $ Alert - It is a session cookie, name doesn't matter
-}))
+    cookie: { httpOnly: false } // It is a session cookie, name doesn't matter
+})) // $ Alert
 
 const http = require('http');
 function test10() {

javascript/ql/test/query-tests/Security/CWE-116/IncompleteSanitization/tst-multi-character-sanitization.js

@@ -88,11 +88,11 @@
 
   x = x.replace(/^(\.\.\/?)+/g, "");
 
-  x = x.replace(/<script\b[^<]*(?:(?!<\/script>)<[^<]*)*<\/script>/g, function( // $ Alert
+  x = x.replace(/<script\b[^<]*(?:(?!<\/script>)<[^<]*)*<\/script>/g, function(
     $0
   ) {
     return unknown ? $0 : "";
-  });
+  }); // $ Alert[js/incomplete-multi-character-sanitization]
 
   x = x.replace(/<\/?([a-z][a-z0-9]*)\b[^>]*>/gi, ""); // $ MISSING: Alert
 

javascript/ql/test/query-tests/Security/CWE-117/logInjectionBad.js

@@ -120,11 +120,11 @@ const server4 = http.createServer((req, res) => {
 
 const serverMatchAll = http.createServer((req, res) => {
     let username = url.parse(req.url, true).query.username;
-    let otherStr = username.matchAll(/.*/g)[0]; // $ Alert
-    console.log(otherStr);
+    let otherStr = username.matchAll(/.*/g)[0];
+    console.log(otherStr); // $ Alert
 });
 
 const serverMatchAl2l = http.createServer((req, res) => {
-    const result = url.parse(req.url, true).query.username.matchAll(/(\d+)/g); // $ Alert
-    console.log("First captured group:", RegExp.$1);
+    const result = url.parse(req.url, true).query.username.matchAll(/(\d+)/g);
+    console.log("First captured group:", RegExp.$1); // $ Alert
 });

javascript/ql/test/query-tests/Security/CWE-1275/tst-sameSite.js

@@ -4,8 +4,8 @@ function esCookies() {
   Cookies.set("authkey", "value", {
     secure: true,
     httpOnly: true,
-    sameSite: "None", // $ Alert
-  });
+    sameSite: "None",
+  }); // $ Alert
 
   Cookies.set("authkey", "value", {
     secure: true,
@@ -21,8 +21,8 @@ function browserCookies() {
     expires: 365,
     secure: true,
     httponly: true,
-    samesite: "None", // $ Alert
-  });
+    samesite: "None",
+  }); // $ Alert
 
   cookies.set("authkey", "value", {
     expires: 365,
@@ -40,7 +40,7 @@ function cookie() {
     httpOnly: true,
     secure: true,
     sameSite: "None",
-  });
+  }); // $ Alert
 
   var setCookie = cookie.serialize("authkey", "value", {
     maxAge: 9000000000,
@@ -59,8 +59,8 @@ app.get("/a", function (req, res, next) {
     maxAge: 9000000000,
     httpOnly: true,
     secure: true,
-    sameSite: "None", // $ Alert
-  });
+    sameSite: "None",
+  }); // $ Alert
 
   res.cookie("session", "value", {
     maxAge: 9000000000,
@@ -78,8 +78,8 @@ app.use(
     keys: ["key1", "key2"],
     httpOnly: true,
     secure: true,
-    sameSite: "None", // $ Alert
-  })
+    sameSite: "None",
+  }) // $ Alert
 );
 
 app.use(
@@ -101,9 +101,9 @@ app.use(
     cookie: {
       httpOnly: true,
       secure: true,
-      sameSite: "None", // $ Alert
+      sameSite: "None",
     },
-  })
+  }) // $ Alert
 );
 
 app.use(

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

@@ -30,11 +30,11 @@ fs.access("myfile", (err) => {
     return;
   }
 
-  fs.open("myfile", "wx", (err, fd) => { // $ Alert
+  fs.open("myfile", "wx", (err, fd) => {
     if (err) throw err;
 
     // ....
-  });
+  }); // $ Alert
 });
 
 const filePath3 = createFile();

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

@@ -6,12 +6,12 @@ app.use(session({
     secret: 'keyboard cat'
 }));
 // handle login
-app.get('/login', function (req, res) { // $ Alert - no regenerate
+app.get('/login', function (req, res) { // no regenerate
     req.session.user = {
         userId: something
     };
     res.send('logged in');
-});
+}); // $ Alert
 
 // with regenerate
 app.get('/login2', function (req, res) {

javascript/ql/test/query-tests/Security/CWE-829/insecure-download.js

@@ -49,8 +49,8 @@ function test() {
 
     nugget("http://example.org/unsafe", {target: "foo.safe"}, () => { })
 
-    $.get("http://example.org/unsafe.unknown", function( data ) {
-        writeFileAtomic('unsafe.exe', data, {}, function (err) {}); // $ Alert
+    $.get("http://example.org/unsafe.unknown", function( data ) { // $ Alert
+        writeFileAtomic('unsafe.exe', data, {}, function (err) {});
     });
 
     $.get("http://example.org/unsafe.unknown", function( data ) {

javascript/ql/test/query-tests/Statements/EphemeralLoop/tst.js

@@ -7,8 +7,8 @@ while(c){ // $ Alert
   break;
 }
 
-function f() { // $ Alert
-  for (; k < numprecincts;) {
+function f() {
+  for (; k < numprecincts;) { // $ Alert
     var packet = createPacket(resolution, k, l);
     k++;
     return packet;

javascript/ql/test/query-tests/Statements/ImplicitReturn/tst.js

@@ -1,5 +1,5 @@
-function f() { // $ Alert
-	if (foo())
+function f() {
+	if (foo()) // $ Alert
 		return true;
 }
 
@@ -95,12 +95,12 @@ function t(n) {
 	} while("true");
 }
 
-var u = function() { // $ Alert
-	if (foo())
+var u = function() {
+	if (foo()) // $ Alert
 		return true;
 };
 
-function v(b) { // $ Alert
-	if (b)
+function v(b) {
+	if (b) // $ Alert
 		return 1;
 }

javascript/ql/test/query-tests/Statements/InconsistentLoopOrientation/tst.js

@@ -2,15 +2,15 @@
 for (j = i - 1; j >= 0; --j) {
 }
 
-for (j = i + 1; j < strLength; --j) { // $ Alert
-}
+for (j = i + 1; j < strLength; --j) {
+} // $ Alert
 
-for (var i = 0, l = c.length; i > l; i ++) { // $ Alert
-}
+for (var i = 0, l = c.length; i > l; i ++) {
+} // $ Alert
 
 
 for (i=lower-1; i>=0; --i)
     a[i] = 0;
 
-for (i=upper+1; i<a.length; --i) // $ Alert
-    a[i] = 0;
+for (i=upper+1; i<a.length; --i)
+    a[i] = 0; // $ Alert

javascript/ql/test/query-tests/Statements/ReturnAssignsLocal/tst.js

@@ -1,10 +1,10 @@
-function f(x) { // $ Alert
-	return x = 23;
+function f(x) {
+	return x = 23; // $ Alert
 }
 
-function g() { // $ Alert
+function g() {
 	var x;
-	return x = 23;
+	return x = 23; // $ Alert
 }
 
 

javascript/ql/test/query-tests/Statements/SuspiciousUnusedLoopIterationVariable/tst.js

@@ -1,6 +1,6 @@
-function countOccurrences(xs, p) { // $ Alert
+function countOccurrences(xs, p) {
 	var count = 0;
-	for (let x of xs)
+	for (let x of xs) // $ Alert
 		if (p())
 			++count;
 	return count;