Use more sensible validator in example.

Author: Max Schaefer

javascript/ql/src/Security/CWE-601/examples/ServerSideUrlRedirect.js

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

javascript/ql/src/Security/CWE-601/examples/ServerSideUrlRedirectGood.js

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

javascript/ql/src/Security/CWE-601/examples/ServerSideUrlRedirectGood2.js

@@ -1,15 +1,15 @@
 const app = require("express")();
 
-function isLocalUrl(url) {
-  return url.startsWith("/") && !url.startsWith("//") && !url.startsWith("/\\");
+function isRelativePath(path) {
+  return !/^(\w+:)?[/\\]{2}/.test(path);
 }
 
-app.get('/redirect', function(req, res) {
+app.get("/redirect", function (req, res) {
   // GOOD: check that we don't redirect to a different host
-  let target = req.params["target"];
-  if (isLocalUrl(target)) {
+  let target = req.query["target"];
+  if (isRelativePath(target)) {
     res.redirect(target);
   } else {
     res.redirect("/");
   }
-});
+});

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

@@ -1,7 +1,7 @@
 nodes
-| ServerSideUrlRedirect.js:5:16:5:35 | req.params["target"] |
-| ServerSideUrlRedirect.js:5:16:5:35 | req.params["target"] |
-| ServerSideUrlRedirect.js:5:16:5:35 | req.params["target"] |
+| ServerSideUrlRedirect.js:5:16:5:34 | req.query["target"] |
+| ServerSideUrlRedirect.js:5:16:5:34 | req.query["target"] |
+| ServerSideUrlRedirect.js:5:16:5:34 | req.query["target"] |
 | express.js:7:16:7:34 | req.param("target") |
 | express.js:7:16:7:34 | req.param("target") |
 | express.js:7:16:7:34 | req.param("target") |
@@ -117,7 +117,7 @@ nodes
 | react-native.js:9:26:9:32 | tainted |
 | react-native.js:9:26:9:32 | tainted |
 edges
-| ServerSideUrlRedirect.js:5:16:5:35 | req.params["target"] | ServerSideUrlRedirect.js:5:16:5:35 | req.params["target"] |
+| ServerSideUrlRedirect.js:5:16:5:34 | req.query["target"] | ServerSideUrlRedirect.js:5:16:5:34 | req.query["target"] |
 | express.js:7:16:7:34 | req.param("target") | express.js:7:16:7:34 | req.param("target") |
 | express.js:12:26:12:44 | req.param("target") | express.js:12:26:12:44 | req.param("target") |
 | express.js:27:7:27:34 | target | express.js:33:18:33:23 | target |
@@ -215,7 +215,7 @@ edges
 | react-native.js:7:17:7:33 | req.param("code") | react-native.js:7:7:7:33 | tainted |
 | react-native.js:7:17:7:33 | req.param("code") | react-native.js:7:7:7:33 | tainted |
 #select
-| ServerSideUrlRedirect.js:5:16:5:35 | req.params["target"] | ServerSideUrlRedirect.js:5:16:5:35 | req.params["target"] | ServerSideUrlRedirect.js:5:16:5:35 | req.params["target"] | Untrusted URL redirection depends on a $@. | ServerSideUrlRedirect.js:5:16:5:35 | req.params["target"] | user-provided value |
+| ServerSideUrlRedirect.js:5:16:5:34 | req.query["target"] | ServerSideUrlRedirect.js:5:16:5:34 | req.query["target"] | ServerSideUrlRedirect.js:5:16:5:34 | req.query["target"] | Untrusted URL redirection depends on a $@. | ServerSideUrlRedirect.js:5:16:5:34 | req.query["target"] | user-provided value |
 | express.js:7:16:7:34 | req.param("target") | express.js:7:16:7:34 | req.param("target") | express.js:7:16:7:34 | req.param("target") | Untrusted URL redirection depends on a $@. | express.js:7:16:7:34 | req.param("target") | user-provided value |
 | express.js:12:26:12:44 | req.param("target") | express.js:12:26:12:44 | req.param("target") | express.js:12:26:12:44 | req.param("target") | Untrusted URL redirection depends on a $@. | express.js:12:26:12:44 | req.param("target") | user-provided value |
 | express.js:33:18:33:23 | target | express.js:27:16:27:34 | req.param("target") | express.js:33:18:33:23 | target | Untrusted URL redirection depends on a $@. | express.js:27:16:27:34 | req.param("target") | user-provided value |

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

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

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

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

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

@@ -1,15 +1,15 @@
 const app = require("express")();
 
-function isLocalUrl(url) {
-  return url.startsWith("/") && !url.startsWith("//") && !url.startsWith("/\\");
+function isRelativePath(path) {
+  return !/^(\w+:)?[/\\]{2}/.test(path);
 }
 
-app.get('/redirect', function(req, res) {
+app.get("/redirect", function (req, res) {
   // GOOD: check that we don't redirect to a different host
-  let target = req.params["target"];
-  if (isLocalUrl(target)) {
+  let target = req.query["target"];
+  if (isRelativePath(target)) {
     res.redirect(target);
   } else {
     res.redirect("/");
   }
-});
+});