Example / Implementation for noSQL Injection

[lirantal]

What do we think about providing an actual example for the classic noSQL injection with MongoDB, as is demonstrated and documented in the tutorial?

Screenshot from A1 - Injection tutorial:

So while the tutorials show this example, the NodeGoat app actually implements the user login differently with:

usersCol.findOne({
            userName: userName
        }, validateUserDoc);

Do we possibly want to change the login to use the classic style as is documented or maybe provide another login screen just for the sake of demo'ing the example?

[ckarande]

@lirantal Thanks for bringing this up..A working example of NoSQL injection with $gt or $ne would be a great addition. I would prefer to modify existing login functionality (instead of another login screen or API) to incorporate this vulnerability. Do you see any issues with that?

[lirantal]

@ckarande @binarymist I don't see any technical issues with changing the current implementation to a naive one as I suggested above. The only down-side is that the current one is still susceptible to blind nosql injections if someone would want to demo that. And also it features other issues that are useful to demo and education about, for example the different user/pass error messages, and lastly the password comparing which also ties to a tutorial section:

    this.validateLogin = function(userName, password, callback) {

        // Helper function to compare passwords
        function comparePassword(fromDB, fromUser) {
            return fromDB === fromUser;
            /*
            // Fix for A2-Broken Auth
            // compares decrypted password stored in this.addUser()
            return bcrypt.compareSync(fromDB, fromUser);
            */
        }

What I could suggest is that we keep the validateLogin function for user login as commented out maybe, and insert the simple one? I'm not really sure on which is the best approach, that is why I leaned towards creating another route so we can demonstrate 2 login implementations at the same time (which both are vulnerable due to different issues in each).

What do you guys say?

[binarymist]

Would adding a feature somewhere else, that leverages/exhibits this defect be a better approach, rather than trying to overload (convolute) the login?

[bilkoh]

If I'm not mistaken, enabling $gt and $ne injection via post requires setting app.use(bodyParser.urlencoded({extended: false})) to true in server.js

Otherwise const { userName, password } = req.body goes undefined and req.body looks like this:
{'userName[$ne]': '', 'password[$ne]': ''}
instead of
{ userName: { '$ne': '' }, password: { '$ne': '' }

Second issue is that because the app is evaluating the password and not mongo, this injection wouldn't bypass the login anyhow.

Anyway, I saw this on the 2019-2020 roadmap and was interested in contributing. My thinking is that if this was to be implemented thru the current login function, you would A) have to switch use the extended urlencoding (I did this and it seems working, but it could be breaking stuff elsewhere) and B) let mongo match both the username and password in .findOne statement.

I see that most of this discussion took place in 2017. I'd like to know more if this is still something that needs contribution.

[lirantal]

That's correct @bilkoh 👍
I still show-case this vulnerability for noSQL injection in my own fork using this branch so it definitely works and viable but we were wondering whether the example as-is makes an interesting flow/story.

We're happy if you want to continue work on this for sure.