diff --git a/app/routes/contributions.js b/app/routes/contributions.js index 4808f06f1..93b5a02a8 100644 --- a/app/routes/contributions.js +++ b/app/routes/contributions.js @@ -26,26 +26,16 @@ function ContributionsHandler(db) { }; this.handleContributionsUpdate = (req, res, next) => { + // Secure parsing of inputs without using eval() + const preTax = parseInt(req.body.preTax, 10); + const afterTax = parseInt(req.body.afterTax, 10); + const roth = parseInt(req.body.roth, 10); - /*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 { userId } = req.session; - /* - //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; - - //validate contributions - const validations = [isNaN(preTax), isNaN(afterTax), isNaN(roth), preTax < 0, afterTax < 0, roth < 0] - const isInvalid = validations.some(validation => validation) + // Validate contributions + const validations = [isNaN(preTax), isNaN(afterTax), isNaN(roth), preTax < 0, afterTax < 0, roth < 0]; + const isInvalid = validations.some(validation => validation); if (isInvalid) { return res.render("contributions", { updateError: "Invalid contribution percentages", @@ -56,14 +46,13 @@ function ContributionsHandler(db) { // Prevent more than 30% contributions if (preTax + afterTax + roth > 30) { return res.render("contributions", { - updateError: "Contribution percentages cannot exceed 30 %", + updateError: "Contribution percentages cannot exceed 30%", userId, environmentalScripts }); } contributionsDAO.update(userId, preTax, afterTax, roth, (err, contributions) => { - if (err) return next(err); contributions.updateSuccess = true; @@ -72,9 +61,6 @@ function ContributionsHandler(db) { environmentalScripts }); }); - }; - } - -module.exports = ContributionsHandler; +module.exports = ContributionsHandler; \ No newline at end of file diff --git a/app/routes/index.js b/app/routes/index.js index 48c888d6e..86fcf00ec 100644 --- a/app/routes/index.js +++ b/app/routes/index.js @@ -71,7 +71,34 @@ const index = (app, db) => { // Handle redirect for learning resources link app.get("/learn", isLoggedIn, (req, res) => { // Insecure way to handle redirects by taking redirect url from query string - return res.redirect(req.query.url); +// Define an allow-list of trusted URLs +const trustedUrls = [ + 'http://www.example.com/', + 'https://www.anothertrusteddomain.com/', + // Add other trusted URLs here +]; + +// Middleware to validate the redirect URL against the allow-list +const validateRedirectUrl = (req, res, next) => { + const redirectUrl = req.query.url; + if (trustedUrls.includes(redirectUrl)) { + next(); // URL is trusted, proceed with the redirect + } else { + // URL is not trusted, handle according to your security policy + // Option 1: Block the redirect and send an error message + // return res.status(400).send('Invalid redirect URL.'); + + // Option 2: Warn the user they are being redirected to an external site + // This could involve rendering a warning page with a link for the user to click + return res.render('externalRedirectWarning', { redirectUrl }); + } +}; + +// Use the middleware in the route that handles the redirect +app.get("/learn", isLoggedIn, validateRedirectUrl, (req, res) => { + // The URL has been validated against the allow-list and is safe to redirect to + return res.redirect(req.query.url); +}); }); // Handle redirect for learning resources link @@ -85,7 +112,22 @@ const index = (app, db) => { const { page } = req.params - return res.render(`tutorial/${page}`, { +// Define an allow list of permitted pages +const allowedPages = ['introduction', 'basics', 'advanced', 'summary']; + +// Validate the 'page' parameter +const page = req.params.page; // or however you get the 'page' parameter from the user input + +// Check if the requested page is in the allow list +if (allowedPages.includes(page)) { + // If the page is allowed, render it + return res.render(`tutorial/${page}`, { + // ... additional context properties for rendering + }); +} else { + // If the page is not allowed, render an error page or redirect to a safe default + return res.status(404).send('Page not found'); // or res.render('error', { error: 'Page not found' }); +} environmentalScripts }); }); @@ -97,4 +139,4 @@ const index = (app, db) => { app.use(ErrorHandler); }; -module.exports = index; +module.exports = index; \ No newline at end of file diff --git a/app/views/allocations.html b/app/views/allocations.html index 3b28d546f..bd0500239 100644 --- a/app/views/allocations.html +++ b/app/views/allocations.html @@ -1,4 +1,6 @@ -{% extends "./layout.html" %} {% block title %}Allocations{% endblock %} {% block content %} +{% extends "./layout.html" %} +{% block title %}Allocations{% endblock %} +{% block content %}