From 876c1e7b77b958479b44b992908aa46bc7b6f664 Mon Sep 17 00:00:00 2001 From: maytlead Date: Tue, 24 Oct 2023 16:57:05 +0200 Subject: [PATCH 01/11] Fix: A1-1 - Remove all eval functions in converting the user inputs - Convert eval to parseInt --- app/routes/contributions.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/app/routes/contributions.js b/app/routes/contributions.js index 7f68170b9..caba798e4 100644 --- a/app/routes/contributions.js +++ b/app/routes/contributions.js @@ -29,16 +29,15 @@ function ContributionsHandler(db) { /*jslint evil: true */ // Insecure use of eval() to parse inputs - const preTax = eval(req.body.preTax); - const afterTax = eval(req.body.afterTax); - const roth = eval(req.body.roth); + // const preTax = eval(req.body.preTax); + // const afterTax = eval(req.body.afterTax); + // const roth = eval(req.body.roth); - /* //Fix for A1 -1 SSJS Injection attacks - uses alternate method to eval const preTax = parseInt(req.body.preTax); const afterTax = parseInt(req.body.afterTax); const roth = parseInt(req.body.roth); - */ + const { userId } = req.session; From 0661f8b3cde588e2659d99971751db00159d5a30 Mon Sep 17 00:00:00 2001 From: maytlead Date: Tue, 24 Oct 2023 18:48:44 +0200 Subject: [PATCH 02/11] Fix: A1-2 - Remove direct using of request data - Add the validation to request data --- app/data/allocations-dao.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/app/data/allocations-dao.js b/app/data/allocations-dao.js index 24d4718c4..5c8f6b224 100644 --- a/app/data/allocations-dao.js +++ b/app/data/allocations-dao.js @@ -60,7 +60,6 @@ const AllocationsDAO = function(db){ const searchCriteria = () => { if (threshold) { - /* // Fix for A1 - 2 NoSQL Injection - escape the threshold parameter properly // Fix this NoSQL Injection which doesn't sanitze the input parameter 'threshold' and allows attackers // to inject arbitrary javascript code into the NoSQL query: @@ -73,10 +72,10 @@ const AllocationsDAO = function(db){ return {$where: `this.userId == ${parsedUserId} && this.stocks > ${parsedThreshold}`}; } throw `The user supplied threshold: ${parsedThreshold} was not valid.`; - */ - return { - $where: `this.userId == ${parsedUserId} && this.stocks > '${threshold}'` - }; + + // return { + // $where: `this.userId == ${parsedUserId} && this.stocks > '${threshold}'` + // }; } return { userId: parsedUserId From 5b7e7dbb7012b831e18abf0527189ba033110a5a Mon Sep 17 00:00:00 2001 From: maytlead Date: Tue, 24 Oct 2023 19:00:58 +0200 Subject: [PATCH 03/11] Fix: A1-3 - Remove direct use user input in logging - Log out the encoded logging context --- app/routes/session.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/routes/session.js b/app/routes/session.js index 3810fb980..01902b89e 100644 --- a/app/routes/session.js +++ b/app/routes/session.js @@ -61,18 +61,18 @@ function SessionHandler(db) { const invalidPasswordErrorMessage = "Invalid password"; if (err) { if (err.noSuchUser) { - console.log("Error: attempt to login with invalid user: ", userName); + // console.log("Error: attempt to login with invalid user: ", userName); // Fix for A1 - 3 Log Injection - encode/sanitize input for CRLF Injection // that could result in log forging: // - Step 1: Require a module that supports encoding - // const ESAPI = require('node-esapi'); + const ESAPI = require('node-esapi'); // - Step 2: Encode the user input that will be logged in the correct context // following are a few examples: // console.log('Error: attempt to login with invalid user: %s', // ESAPI.encoder().encodeForHTML(userName)); - // console.log('Error: attempt to login with invalid user: %s', - // ESAPI.encoder().encodeForJavaScript(userName)); + console.log('Error: attempt to login with invalid user: %s', + ESAPI.encoder().encodeForJavaScript(userName)); // console.log('Error: attempt to login with invalid user: %s', // ESAPI.encoder().encodeForURL(userName)); // or if you know that this is a CRLF vulnerability you can target this specifically as follows: From b07c41fd8077022faecceda3749dc67d7bacc1d0 Mon Sep 17 00:00:00 2001 From: maytlead Date: Wed, 25 Oct 2023 10:52:39 +0200 Subject: [PATCH 04/11] Fix: A1-2 - Change input type number --- app/views/allocations.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/allocations.html b/app/views/allocations.html index 3b28d546f..11de3ca40 100644 --- a/app/views/allocations.html +++ b/app/views/allocations.html @@ -18,8 +18,8 @@

Adhering to defence in depth, on the front-end mostly for UX. The attacker, or user should not be able to enter anything other than 0-99. Also implement fix in allocations-dao.js--> - - + +

Using above threshold value, it will return all assets allocation above the specified stocks percentage number.

From 4fa0232a60d3875bdf6df2baf2f42b91d0d67a3c Mon Sep 17 00:00:00 2001 From: maytlead Date: Wed, 25 Oct 2023 15:18:03 +0200 Subject: [PATCH 05/11] Fix: A2-1 Session Management - Use hashed password for seeding the data - Before saving the password, hash it - Compare the password with hash method --- app/data/user-dao.js | 8 ++------ artifacts/db-reset.js | 12 ++++++------ 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/app/data/user-dao.js b/app/data/user-dao.js index a674363ef..2bbafbddc 100644 --- a/app/data/user-dao.js +++ b/app/data/user-dao.js @@ -22,12 +22,10 @@ function UserDAO(db) { firstName, lastName, benefitStartDate: this.getRandomFutureDate(), - password //received from request param - /* + // password //received from request param // Fix for A2-1 - Broken Auth // Stores password in a safer way using one way encryption and salt hashing password: bcrypt.hashSync(password, bcrypt.genSaltSync()) - */ }; // Add email if set @@ -58,12 +56,10 @@ function UserDAO(db) { // Helper function to compare passwords const comparePassword = (fromDB, fromUser) => { - return fromDB === fromUser; - /* + // return fromDB === fromUser; // Fix for A2-Broken Auth // compares decrypted password stored in this.addUser() return bcrypt.compareSync(fromDB, fromUser); - */ }; // Callback to pass to MongoDB that validates a user document diff --git a/artifacts/db-reset.js b/artifacts/db-reset.js index 8b79c1191..44d9e933b 100644 --- a/artifacts/db-reset.js +++ b/artifacts/db-reset.js @@ -15,8 +15,8 @@ const USERS_TO_INSERT = [ "userName": "admin", "firstName": "Node Goat", "lastName": "Admin", - "password": "Admin_123", - //"password" : "$2a$10$8Zo/1e8KM8QzqOKqbDlYlONBOzukWXrM.IiyzqHRYDXqwB3gzDsba", // Admin_123 + // "password": "Admin_123", + "password" : "$2a$10$8Zo/1e8KM8QzqOKqbDlYlONBOzukWXrM.IiyzqHRYDXqwB3gzDsba", // Admin_123 "isAdmin": true }, { "_id": 2, @@ -24,16 +24,16 @@ const USERS_TO_INSERT = [ "firstName": "John", "lastName": "Doe", "benefitStartDate": "2030-01-10", - "password": "User1_123" - // "password" : "$2a$10$RNFhiNmt2TTpVO9cqZElb.LQM9e1mzDoggEHufLjAnAKImc6FNE86",// User1_123 + // "password": "User1_123" + "password" : "$2a$10$RNFhiNmt2TTpVO9cqZElb.LQM9e1mzDoggEHufLjAnAKImc6FNE86",// User1_123 }, { "_id": 3, "userName": "user2", "firstName": "Will", "lastName": "Smith", "benefitStartDate": "2025-11-30", - "password": "User2_123" - //"password" : "$2a$10$Tlx2cNv15M0Aia7wyItjsepeA8Y6PyBYaNdQqvpxkIUlcONf1ZHyq", // User2_123 + // "password": "User2_123" + "password" : "$2a$10$Tlx2cNv15M0Aia7wyItjsepeA8Y6PyBYaNdQqvpxkIUlcONf1ZHyq", // User2_123 }]; const tryDropCollection = (db, name) => { From a6c1f670a6f193095b41e22095fe6fcd416b48da Mon Sep 17 00:00:00 2001 From: maytlead Date: Wed, 25 Oct 2023 16:05:12 +0200 Subject: [PATCH 06/11] Fix: A2-2 Password Protection - Add password protection using length, complexity --- app/routes/session.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/routes/session.js b/app/routes/session.js index 01902b89e..2bdf3834c 100644 --- a/app/routes/session.js +++ b/app/routes/session.js @@ -141,12 +141,10 @@ function SessionHandler(db) { const FNAME_RE = /^.{1,100}$/; const LNAME_RE = /^.{1,100}$/; const EMAIL_RE = /^[\S]+@[\S]+\.[\S]+$/; - const PASS_RE = /^.{1,20}$/; - /* + // const PASS_RE = /^.{1,20}$/; //Fix for A2-2 - Broken Authentication - requires stronger password //(at least 8 characters with numbers and both lowercase and uppercase letters.) const PASS_RE =/^(?=.*\d)(?=.*[a-z])(?=.*[A-Z]).{8,}$/; - */ errors.userNameError = ""; errors.firstNameError = ""; From e92abcc6ace9c86b081f14f6389dfe6c3a63a15d Mon Sep 17 00:00:00 2001 From: maytlead Date: Wed, 25 Oct 2023 15:32:37 +0200 Subject: [PATCH 07/11] Fix: A2-1 Session Management - Regenerating the session whenever the user log in --- app/routes/session.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/routes/session.js b/app/routes/session.js index 3810fb980..10d7274d0 100644 --- a/app/routes/session.js +++ b/app/routes/session.js @@ -112,9 +112,10 @@ function SessionHandler(db) { // Fix the problem by regenerating a session in each login // by wrapping the below code as a function callback for the method req.session.regenerate() // i.e: - // `req.session.regenerate(() => {})` - req.session.userId = user._id; - return res.redirect(user.isAdmin ? "/benefits" : "/dashboard"); + req.session.regenerate(() => { + req.session.userId = user._id; + return res.redirect(user.isAdmin ? "/benefits" : "/dashboard"); + }); }); }; From a1846fb9fb5abd2279bff7e2b50517763df40b55 Mon Sep 17 00:00:00 2001 From: maytlead Date: Wed, 25 Oct 2023 16:13:10 +0200 Subject: [PATCH 08/11] Fix: A2-2 Password Protection - Use idential error message for both username and password to not expose what is wrong to the attacker --- app/routes/session.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/routes/session.js b/app/routes/session.js index 2bdf3834c..8d69f1b3b 100644 --- a/app/routes/session.js +++ b/app/routes/session.js @@ -82,18 +82,18 @@ function SessionHandler(db) { return res.render("login", { userName: userName, password: "", - loginError: invalidUserNameErrorMessage, + // loginError: invalidUserNameErrorMessage, //Fix for A2-2 Broken Auth - Uses identical error for both username, password error - // loginError: errorMessage + loginError: errorMessage, environmentalScripts }); } else if (err.invalidPassword) { return res.render("login", { userName: userName, password: "", - loginError: invalidPasswordErrorMessage, + // loginError: invalidPasswordErrorMessage, //Fix for A2-2 Broken Auth - Uses identical error for both username, password error - // loginError: errorMessage + loginError: errorMessage, environmentalScripts }); } else { From 1a8d308479643b6585ebcfef8c81d9630b5baa8d Mon Sep 17 00:00:00 2001 From: maytlead Date: Thu, 26 Oct 2023 02:43:20 +0200 Subject: [PATCH 09/11] Fix: A3 XSS Attack - Make cookie only accessible via http to prevent XSS attack --- server.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/server.js b/server.js index d6bb500a2..7cbeb6088 100644 --- a/server.js +++ b/server.js @@ -82,22 +82,20 @@ MongoClient.connect(db, (err, db) => { secret: cookieSecret, // Both mandatory in Express v4 saveUninitialized: true, - resave: true + resave: true, /* // Fix for A5 - Security MisConfig // Use generic cookie name key: "sessionId", */ - /* // Fix for A3 - XSS // TODO: Add "maxAge" cookie: { - httpOnly: true + httpOnly: true, // Remember to start an HTTPS server to get this working - // secure: true + secure: true } - */ })); From 2f68b1851c1f243b10284b35a922edc59ce39f27 Mon Sep 17 00:00:00 2001 From: maytlead Date: Thu, 26 Oct 2023 02:45:09 +0200 Subject: [PATCH 10/11] Fix: A3 XSS Attack - Run the server as https --- server.js | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/server.js b/server.js index 7cbeb6088..5853886d1 100644 --- a/server.js +++ b/server.js @@ -9,13 +9,13 @@ const consolidate = require("consolidate"); // Templating library adapter for Ex const swig = require("swig"); // const helmet = require("helmet"); const MongoClient = require("mongodb").MongoClient; // Driver for connecting to MongoDB -const http = require("http"); +// const http = require("http"); const marked = require("marked"); //const nosniff = require('dont-sniff-mimetype'); const app = express(); // Web framework to handle routing requests const routes = require("./app/routes"); const { port, db, cookieSecret } = require("./config/config"); // Application config properties -/* + // Fix for A6-Sensitive Data Exposure // Load keys for establishing secure HTTPS connection const fs = require("fs"); @@ -25,7 +25,6 @@ const httpsOptions = { key: fs.readFileSync(path.resolve(__dirname, "./artifacts/cert/server.key")), cert: fs.readFileSync(path.resolve(__dirname, "./artifacts/cert/server.crt")) }; -*/ MongoClient.connect(db, (err, db) => { if (err) { @@ -140,16 +139,14 @@ MongoClient.connect(db, (err, db) => { }); // Insecure HTTP connection - http.createServer(app).listen(port, () => { - console.log(`Express http server listening on port ${port}`); - }); + // http.createServer(app).listen(port, () => { + // console.log(`Express http server listening on port ${port}`); + // }); - /* // Fix for A6-Sensitive Data Exposure // Use secure HTTPS protocol https.createServer(httpsOptions, app).listen(port, () => { console.log(`Express http server listening on port ${port}`); }); - */ }); From 260012211b96c862c0be11b03216b15904db31aa Mon Sep 17 00:00:00 2001 From: maytlead Date: Thu, 26 Oct 2023 02:51:45 +0200 Subject: [PATCH 11/11] Fix: A3 XSS - Enable autoescape for XSS --- server.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/server.js b/server.js index 5853886d1..d547b75b2 100644 --- a/server.js +++ b/server.js @@ -131,11 +131,9 @@ MongoClient.connect(db, (err, db) => { // Template system setup swig.setDefaults({ // Autoescape disabled - autoescape: false - /* + // autoescape: false // Fix for A3 - XSS, enable auto escaping autoescape: true // default value - */ }); // Insecure HTTP connection