Skip to content

Patched results for branch: master #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Patched results for branch: master #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 10 additions & 24 deletions app/routes/contributions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Author

@patched-codes patched-codes bot Nov 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CWE-95: Improper Neutralization of Directives in Dynamically Evaluated Code ('Eval Injection')

Detected the use of eval(). eval() can be dangerous if used to evaluate dynamic content. If this content can be input from outside the program, this may be a code injection vulnerability. Ensure evaluated content is not definable by external sources.

const afterTax = eval(req.body.afterTax);
Copy link
Author

@patched-codes patched-codes bot Nov 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CWE-95: Improper Neutralization of Directives in Dynamically Evaluated Code ('Eval Injection')

Detected the use of eval(). eval() can be dangerous if used to evaluate dynamic content. If this content can be input from outside the program, this may be a code injection vulnerability. Ensure evaluated content is not definable by external sources.

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",
Expand All @@ -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;
Expand All @@ -72,9 +61,6 @@ function ContributionsHandler(db) {
environmentalScripts
});
});

};

}

module.exports = ContributionsHandler;
module.exports = ContributionsHandler;
48 changes: 45 additions & 3 deletions app/routes/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Author

@patched-codes patched-codes bot Nov 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CWE-601: URL Redirection to Untrusted Site ('Open Redirect')

The application redirects to a URL specified by user-supplied input req that is not validated. This could redirect users to malicious locations. Consider using an allow-list approach to validate URLs, or warn users they are being redirected to a third-party website.

// 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
Expand All @@ -85,7 +112,22 @@ const index = (app, db) => {
const {
page
} = req.params
return res.render(`tutorial/${page}`, {
Copy link
Author

@patched-codes patched-codes bot Nov 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CWE-706: Use of Incorrectly-Resolved Name or Reference

User controllable data req enters res.render(...) this can lead to the loading of other HTML/templating pages that they may not be authorized to render. An attacker may attempt to use directory traversal techniques e.g. ../folder/index to access other HTML pages on the file system. Where possible, do not allow users to define what should be loaded in res.render or use an allow list for the existing application.

// 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
});
});
Expand All @@ -97,4 +139,4 @@ const index = (app, db) => {
app.use(ErrorHandler);
};

module.exports = index;
module.exports = index;
12 changes: 8 additions & 4 deletions app/views/allocations.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
{% extends "./layout.html" %} {% block title %}Allocations{% endblock %} {% block content %}
{% extends "./layout.html" %}
{% block title %}Allocations{% endblock %}
{% block content %}

<div class="row">
<div class="col-lg-12">
Expand All @@ -12,14 +14,14 @@ <h3 class="panel-title">

<div class="panel-body">

<form action="/allocations/{{userId}}" method="get" role="search">
<form action="/allocations/{{userId}}" method="post" role="search">
{% csrf_token %}
<div class="form-group">
<!--Fix for A1 - 2 NoSQL Injection - Provide validation for input.
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-->
<!--<input type="number" min="0" max="99" class="form-control" placeholder="Stocks Threshold" name="threshold" />-->
<input type="text" class="form-control" placeholder="Stocks Threshold" name="threshold" />
<input type="number" min="0" max="99" class="form-control" placeholder="Stocks Threshold" name="threshold" />
<p class="help-block">Using above threshold value, it will return all assets allocation above the specified stocks percentage number.</p>
</div>

Copy link
Author

@patched-codes patched-codes bot Nov 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CWE-352: Cross-Site Request Forgery (CSRF)

Manually-created forms in django templates should specify a csrf_token to prevent CSRF attacks

Expand Down Expand Up @@ -50,4 +52,6 @@ <h3 class="panel-title">

</div>
</div>

{% endblock %}
{% endblock %}
21 changes: 11 additions & 10 deletions app/views/benefits.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,17 @@
<td>{{user.firstName}}</td>
<td>{{user.lastName}}</td>
<td>
<form method="POST" action="/benefits">
<div class="input-group">
<input type="hidden" name="userId" value="{{user._id.toString()}}"></input>
<input type="date" class="form-control" name="benefitStartDate" value="{{user.benefitStartDate}}"></input>
<span class="input-group-btn">
<button type="submit" class="btn btn-default">Save</button>
</span>
</div>
<!-- /input-group -->
</form>
Copy link
Author

@patched-codes patched-codes bot Nov 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CWE-352: Cross-Site Request Forgery (CSRF)

Manually-created forms in django templates should specify a csrf_token to prevent CSRF attacks

<form method="POST" action="/benefits">
{% csrf_token %}
<div class="input-group">
<input type="hidden" name="userId" value="{{ user._id.toString }}"></input>
<input type="date" class="form-control" name="benefitStartDate" value="{{ user.benefitStartDate }}"></input>
<span class="input-group-btn">
<button type="submit" class="btn btn-default">Save</button>
</span>
</div>
<!-- /input-group -->
</form>
</td>
</tr>
{% endfor %}
Expand Down
41 changes: 20 additions & 21 deletions app/views/login.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,27 +104,26 @@



<form method="post" role="form" method="post" id="loginform">
<div class="form-group">
<label for="userName">User Name</label>
<input type="text" class="form-control" id="userName" name="userName" value="{{userName}}" placeholder="Enter User Name">
</div>

<div class="form-group">
<label for="password">Password</label>
<input type="password" class="form-control" id="password" name="password" value="{{password}}" placeholder="Enter Password">
</div>
<input type="hidden" name="_csrf" value="{{csrftoken}}" />

<div class="row">
<div class="col-lg-4"><a href="/signup">New user? Sign Up</a>
</div>
<div class="col-lg-5"></div>
<div class="col-lg-3">
<button type="submit" class="btn btn-danger">Submit</button>
</div>
</div>
</form>
Copy link
Author

@patched-codes patched-codes bot Nov 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CWE-352: Cross-Site Request Forgery (CSRF)

Manually-created forms in django templates should specify a csrf_token to prevent CSRF attacks

<form method="post" role="form" id="loginform">
{% csrf_token %}
<div class="form-group">
<label for="userName">User Name</label>
<input type="text" class="form-control" id="userName" name="userName" value="{{ userName }}" placeholder="Enter User Name">
</div>

<div class="form-group">
<label for="password">Password</label>
<input type="password" class="form-control" id="password" name="password" value="{{ password }}" placeholder="Enter Password">
</div>

<div class="row">
<div class="col-lg-4"><a href="/signup">New user? Sign Up</a></div>
<div class="col-lg-5"></div>
<div class="col-lg-3">
<button type="submit" class="btn btn-danger">Submit</button>
</div>
</div>
</form>
</div>
</div>
</div>
Expand Down
8 changes: 6 additions & 2 deletions app/views/memos.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
{% extends "./layout.html" %} {% block title %}Memos{% endblock %} {% block content %}
{% extends "./layout.html" %}
{% block title %}Memos{% endblock %}
{% block content %}

<div class="row">
<div class="col-lg-12">
Expand All @@ -13,7 +15,7 @@ <h3 class="panel-title">
<div class="panel-body">

<form action="/memos" method="post" role="search">

{% csrf_token %}
<div class="form-group">
<textarea class="form-control" name="memo"></textarea>
<p class="help-block">You may use Markdown syntax to format your memo</p>
Copy link
Author

@patched-codes patched-codes bot Nov 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CWE-352: Cross-Site Request Forgery (CSRF)

Manually-created forms in django templates should specify a csrf_token to prevent CSRF attacks

Expand All @@ -35,4 +37,6 @@ <h3 class="panel-title">

</div>
</div>

{% endblock %}
{% endblock %}
3 changes: 2 additions & 1 deletion app/views/profile.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ <h3 class="panel-title">Edit Profile</h3>

<!-- @FIXME use a properly escaped variable that matches the URL context, for example
refer to a firstNameSafeURLString field on the doc object set by the controller for this template -->
<a href="{{firstNameSafeString}}">Google search this profile by name</a>
Copy link
Author

@patched-codes patched-codes bot Nov 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')

Detected a template variable used in an anchor tag with the 'href' attribute. This allows a malicious actor to input the 'javascript:' URI and is subject to cross- site scripting (XSS) attacks. If using a relative URL, start with a literal forward slash and concatenate the URL, like this: href='/{{link}}'. You may also consider setting the Content Security Policy (CSP) header.

<!-- Corrected anchor tag with proper URL encoding and concatenation -->
<a href="/search?name={{firstNameSafeURLString}}">Google search this profile by name</a>
</form>
</div>
</div>
Expand Down
11 changes: 9 additions & 2 deletions app/views/research.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
{% extends "./layout.html" %} {% block title %}Research{% endblock %} {% block content %}
{% extends "./layout.html" %}
{% block title %}Research{% endblock %}
{% block content %}

<div class="row">
<div class="col-lg-12">
Expand All @@ -12,7 +14,9 @@ <h3 class="panel-title">

<div class="panel-body">

<form action="/research" method="get" role="search">
<!-- Begin form with CSRF token -->
<form action="/research" method="post" role="search">
{% csrf_token %}
<div class="form-group">
<input type="hidden" id="url" name="url" value="https://finance.yahoo.com/quote/"/>
<input type="text" class="form-control" placeholder="Stock Symbol" name="symbol" />
Expand All @@ -21,10 +25,13 @@ <h3 class="panel-title">

<button type="submit" class="btn btn-primary">Submit</button>
</form>
Copy link
Author

@patched-codes patched-codes bot Nov 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CWE-352: Cross-Site Request Forgery (CSRF)

Manually-created forms in django templates should specify a csrf_token to prevent CSRF attacks

<!-- End form with CSRF token -->

</div>
</div>

</div>
</div>

{% endblock %}
{% endblock %}