Adding a NoSQL Injection vulnerability

[lirantal]

This PR adds a malicious NoSQL Injection vulnerability tagged as A-2 to the allocations view.
It adds an insecure use of input parameters from a GET request that is used to filter the allocations view results only for specific items.

If it looks good I will accompany it with documentation updates to the docs/tutorial.

[ckarande]

Great! Thanks @lirantal . I will review and merge the PR soon.

[lirantal]

Thanks @ckarande
I'll go ahead and update the tutorial with this example and description about this vulnerability so you can review it as well.

[lirantal]

Hey @ckarande, I updated the PR with documentation for the tutorial.
You're welcome to take a peek and merge on your convenience :)

[ckarande]

@lirantal , Thank you very much for the PR and updated documentation. The changes to the code look good. I had a few suggetions / questions -

To make the vulnerability easier and bit more obivious for users to exploit, is it possible to enhance it a bit - instead of setting the threshold to 2, can we create a text field for that? May be we can move this new textfield to the top (above Domestic Stocks label). It would be even better if we could allow users to select the type of asset to filter on instead of limiting it to stocks by providing a dropdown for asset types. Depending on the threshold we can hide / show the assets.

Please let me know your thoughts.

[lirantal]

@ckarande sure, good comments. I'll think about how to better layout the UI there.

[lirantal]

@ckarande what do you think about this one?

Adding a drop down per stock might be confusing and I don't see much value in there. WDYT?

[ckarande]

Excellent 👍

[lirantal]

@ckarande I updated the PR.
I made sure that the title specifies Stocks Assets filtering so it is simple to understand.

[ckarande]

Thanks @lirantal .. What inputs you use for filter to craft the injection attack?

[lirantal]

@ckarande use this query to trigger a DoS attack on the MongoDB:

http://localhost:4000/allocations-threshold?threshold=5';while(true){};'

[lirantal]

Thanks!

[binarymist]

I'm just wondering if it would make sense or be more in keeping with the existing style of the app to merge:

app.get("/allocations/:userId", isLoggedIn, allocationsHandler.displayAllocations);
// With
app.get("/allocations-threshold", isLoggedIn, allocationsHandler.displayAllocationsThreshold);

In app/routes/index.js

2. displayAllocations with displayAllocationsThreshold
   in app/routes/allocations.js

3. getByUserId with getThresholdByUserId
   in app/data/allocations-dao.js

and in the comments in app/data/allocations-dao.js provide a specific fix like I think most/all of the other OWASP Top 10 defects commented in code supply?

PR #85 Addresses this.

[lirantal]

Sure I don't mind.
@ckarande WDYT? if you confirm I'll be happy to open a PR.

[ckarande]

@binarymist 👍 . These changes make sense and keeps the style consistent.

[lirantal]

Thanks!