Merge pull request #3483 from esbena/js/fix-qhelp-FNs

Approved by asgerf

javascript/ql/src/Security/CWE-079/examples/XssThroughDom.js

@@ -1,4 +1,4 @@
 $("button").click(function () {
-    var target = this.attr("data-target");
+    var target = $(this).attr("data-target");
     $(target).hide();
 });

javascript/ql/src/Security/CWE-079/examples/XssThroughDomFixed.js

@@ -1,4 +1,4 @@
 $("button").click(function () {
-    var target = this.attr("data-target");
-    $.find(target).hide();
+    var target = $(this).attr("data-target");
+	$.find(target).hide();
 });

javascript/ql/src/Security/CWE-089/examples/SqlInjection.js

@@ -1,18 +1,21 @@
-const pg = require('pg');
-const pool = new pg.Pool(config);
+const app = require("express")(),
+      pg = require("pg"),
+      pool = new pg.Pool(config);
 
-function handler(req, res) {
+app.get("search", function handler(req, res) {
   // BAD: the category might have SQL special characters in it
-  var query1 = "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='"
-             + req.params.category + "' ORDER BY PRICE";
+  var query1 =
+    "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='" +
+    req.params.category +
+    "' ORDER BY PRICE";
   pool.query(query1, [], function(err, results) {
     // process results
   });
 
   // GOOD: use parameters
-  var query2 = "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY=$1"
-             + " ORDER BY PRICE";
+  var query2 =
+    "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY=$1" + " ORDER BY PRICE";
   pool.query(query2, [req.params.category], function(err, results) {
-      // process results
+    // process results
   });
-}
+});

javascript/ql/src/Security/CWE-134/examples/TaintedFormatStringBad.js

@@ -1 +1,7 @@
-console.log("Unauthorized access attempt by " + user, ip);
+const app = require("express")();
+
+app.get("unauthorized", function handler(req, res) {
+  let user = req.query.user;
+  let ip = req.connection.remoteAddress;
+  console.log("Unauthorized access attempt by " + user, ip);
+});

javascript/ql/src/Security/CWE-134/examples/TaintedFormatStringGood.js

@@ -1 +1,7 @@
-console.log("Unauthorized access attempt by %s", user, ip);
+const app = require("express")();
+
+app.get("unauthorized", function handler(req, res) {
+  let user = req.query.user;
+  let ip = req.connection.remoteAddress;
+  console.log("Unauthorized access attempt by %s", user, ip);
+});

javascript/ql/src/Security/CWE-327/examples/BrokenCryptoAlgorithm.js

@@ -1,7 +1,9 @@
 const crypto = require('crypto');
 
+var secretText = obj.getSecretText();
+
 const desCipher = crypto.createCipher('des', key);
-let desEncrypted = cipher.write(secretText, 'utf8', 'hex'); // BAD: weak encryption
+let desEncrypted = desCipher.write(secretText, 'utf8', 'hex'); // BAD: weak encryption
 
 const aesCipher = crypto.createCipher('aes-128', key);
-let aesEncrypted = cipher.update(secretText, 'utf8', 'hex'); // GOOD: strong encryption
+let aesEncrypted = aesCipher.update(secretText, 'utf8', 'hex'); // GOOD: strong encryption

javascript/ql/src/Security/CWE-352/examples/MissingCsrfMiddlewareBad.js

@@ -1,11 +1,11 @@
-var express = require('express')
-var cookieParser = require('cookie-parser')
-var passport = require('passport')
+var app = require("express")(),
+  cookieParser = require("cookie-parser"),
+  passport = require("passport");
 
-var app = express()
+app.use(cookieParser());
+app.use(passport.authorize({ session: true }));
 
-app.use(cookieParser())
-app.use(passport.authorize({ session: true }))
-
-app.post('/changeEmail', ..., function (req, res) {
-})
+app.post("/changeEmail", function(req, res) {
+  let newEmail = req.cookies["newEmail"];
+  // ...
+});

javascript/ql/src/Security/CWE-352/examples/MissingCsrfMiddlewareGood.js

@@ -1,13 +1,12 @@
-var express = require('express')
-var cookieParser = require('cookie-parser')
-var passport = require('passport')
-var csrf = require('csurf')
+var app = require("express")(),
+  cookieParser = require("cookie-parser"),
+  passport = require("passport"),
+  csrf = require("csurf");
 
-var app = express()
-
-app.use(cookieParser())
-app.use(passport.authorize({ session: true }))
-app.use(csrf({ cookie:true }))
-
-app.post('/changeEmail', ..., function (req, res) {
-})
+app.use(cookieParser());
+app.use(passport.authorize({ session: true }));
+app.use(csrf({ cookie: true }));
+app.post("/changeEmail", function(req, res) {
+  let newEmail = req.cookies["newEmail"];
+  // ...
+});

javascript/ql/src/Security/CWE-502/examples/UnsafeDeserializationBad.js

@@ -1,6 +1,7 @@
-const jsyaml = require("js-yaml");
+const app = require("express")(),
+  jsyaml = require("js-yaml");
 
-function requestHandler(req, res) {
+app.get("load", function(req, res) {
   let data = jsyaml.load(req.params.data);
   // ...
-}
+});

javascript/ql/src/Security/CWE-502/examples/UnsafeDeserializationGood.js

@@ -1,6 +1,7 @@
-const jsyaml = require("js-yaml");
+const app = require("express")(),
+  jsyaml = require("js-yaml");
 
-function requestHandler(req, res) {
+app.get("load", function(req, res) {
   let data = jsyaml.safeLoad(req.params.data);
   // ...
-}
+});

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

@@ -1,3 +1,5 @@
+const app = require("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"));

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

@@ -1,3 +1,5 @@
+const app = require("express")();
+
 const VALID_REDIRECT = "http://cwe.mitre.org/data/definitions/601.html";
 
 app.get('/some/path', function(req, res) {

javascript/ql/src/Security/CWE-611/examples/Xxe.js

@@ -1,2 +1,7 @@
-const libxml = require('libxmljs');
-var doc = libxml.parseXml(xmlSrc, { noent: true });
+const app = require("express")(),
+  libxml = require("libxmljs");
+
+app.post("upload", (req, res) => {
+  let xmlSrc = req.body,
+    doc = libxml.parseXml(xmlSrc, { noent: true });
+});

javascript/ql/src/Security/CWE-611/examples/XxeGood.js

@@ -1,2 +1,7 @@
-const libxml = require('libxmljs');
-var doc = libxml.parseXml(xmlSrc);
+const app = require("express")(),
+  libxml = require("libxmljs");
+
+app.post("upload", (req, res) => {
+  let xmlSrc = req.body,
+    doc = libxml.parseXml(xmlSrc);
+});

javascript/ql/src/Security/CWE-776/examples/XmlBomb.js

@@ -1,5 +1,10 @@
-const expat = require('node-expat');
-var parser = new expat.Parser();
-parser.on('startElement', handleStart);
-parser.on('text', handleText);
-parser.write(xmlSrc);
+const app = require("express")(),
+  expat = require("node-expat");
+
+app.post("upload", (req, res) => {
+  let xmlSrc = req.body,
+    parser = new expat.Parser();
+  parser.on("startElement", handleStart);
+  parser.on("text", handleText);
+  parser.write(xmlSrc);
+});

javascript/ql/src/Security/CWE-776/examples/XmlBombGood.js

@@ -1,5 +1,10 @@
-const sax = require('sax');
-var parser = sax.parser(true);
-parser.onopentag = handleStart;
-parser.ontext = handleText;
-parser.write(xmlSrc);
+const app = require("express")(),
+  sax = require("sax");
+
+app.post("upload", (req, res) => {
+  let xmlSrc = req.body,
+    parser = sax.parser(true);
+  parser.onopentag = handleStart;
+  parser.ontext = handleText;
+  parser.write(xmlSrc);
+});

javascript/ql/src/Security/CWE-798/examples/HardcodedCredentials.js

@@ -1,10 +1,10 @@
-const pg = require('pg')
+const pg = require("pg");
 
 const client = new pg.Client({
-  user: 'dbuser',
-  host: 'database.server.com',
-  database: 'mydb',
-  password: 'secretpassword',
-  port: 3211,
-})
-client.connect()
+  user: "bob",
+  host: "database.server.com",
+  database: "mydb",
+  password: "correct-horse-battery-staple",
+  port: 3211
+});
+client.connect();

javascript/ql/src/Security/CWE-843/examples/TypeConfusionThroughParameterTampering.js

@@ -1,15 +1,15 @@
-var express = require('express'),
-    path = require('path'),
-var app = express();
+var app = require("express")(),
+  path = require("path");
 
-app.get('/user-files', function(req, res) {
-    var file = req.param('file');
-    if (file.indexOf('..') !== -1) { // BAD
-        // forbid paths outside the /public directory
-        res.status(400).send('Bad request');
-    } else {
-        var absolute = path.resolve('/public/' + file);
-        console.log("Sending file: %s", absolute);
-        res.sendFile(absolute);
-    }
+app.get("/user-files", function(req, res) {
+  var file = req.param("file");
+  if (file.indexOf("..") !== -1) {
+    // BAD
+    // forbid paths outside the /public directory
+    res.status(400).send("Bad request");
+  } else {
+    var absolute = path.resolve("/public/" + file);
+    console.log("Sending file: %s", absolute);
+    res.sendFile(absolute);
+  }
 });

javascript/ql/src/Security/CWE-843/examples/TypeConfusionThroughParameterTampering_fixed.js

@@ -1,15 +1,15 @@
-var express = require('express'),
-    path = require('path'),
-var app = express();
+var app = require("express")(),
+  path = require("path");
 
-app.get('/user-files', function(req, res) {
-    var file = req.param('file');
-    if (typeof path !== 'string' || path.indexOf('..') !== -1) { // GOOD
-        // forbid paths outside the /public directory
-        res.status(400).send('Bad request');
-    } else {
-        var full = path.resolve('/public/' + file);
-        console.log("Sending file: %s", full);
-        res.sendFile(full);
-    }
+app.get("/user-files", function(req, res) {
+  var file = req.param("file");
+  if (typeof path !== 'string' || file.indexOf("..") !== -1) {
+    // BAD
+    // forbid paths outside the /public directory
+    res.status(400).send("Bad request");
+  } else {
+    var absolute = path.resolve("/public/" + file);
+    console.log("Sending file: %s", absolute);
+    res.sendFile(absolute);
+  }
 });

javascript/ql/src/Security/CWE-916/examples/InsufficientPasswordHash.js

@@ -1,5 +1,5 @@
+const crypto = require("crypto");
 function hashPassword(password) {
-    var crypto = require("crypto");
     var hasher = crypto.createHash('md5');
     var hashed = hasher.update(password).digest("hex"); // BAD
     return hashed;

javascript/ql/src/Security/CWE-916/examples/InsufficientPasswordHash_fixed.js

@@ -1,5 +1,5 @@
+const bcrypt = require("bcrypt");
 function hashPassword(password, salt) {
-    var bcrypt = require('bcrypt');
-    var hashed = bcrypt.hashSync(password, salt); // GOOD
-    return hashed;
+  var hashed = bcrypt.hashSync(password, salt); // GOOD
+  return hashed;
 }

javascript/ql/src/Security/CWE-918/examples/RequestForgeryBad.js

@@ -2,7 +2,7 @@ import http from 'http';
 import url from 'url';
 
 var server = http.createServer(function(req, res) {
-    var target = url.parse(request.url, true).query.target;
+    var target = url.parse(req.url, true).query.target;
 
     // BAD: `target` is controlled by the attacker
     http.get('https://' + target + ".example.com/data/", res => {

javascript/ql/src/Security/CWE-918/examples/RequestForgeryGood.js

@@ -2,7 +2,7 @@ import http from 'http';
 import url from 'url';
 
 var server = http.createServer(function(req, res) {
-    var target = url.parse(request.url, true).query.target;
+    var target = url.parse(req.url, true).query.target;
 
     var subdomain;
     if (target === 'EU') {