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 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/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; diff --git a/app/routes/session.js b/app/routes/session.js index 3810fb980..63703536e 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: @@ -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 { @@ -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"); + }); }); }; @@ -141,12 +142,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 = ""; 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.

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) => { diff --git a/server.js b/server.js index d6bb500a2..d547b75b2 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) { @@ -82,22 +81,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 } - */ })); @@ -134,24 +131,20 @@ 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 - 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}`); }); - */ });