DEV Community

Cover image for Beware of the findOne function
Igor Souza Martins
Igor Souza Martins

Posted on

Beware of the findOne function

Hello, since my last post Easy Requests in NodeJS, I moved to the information security industry and started to study / investigate a lot about vulnerabilities in modern applications.

In this post, we will find how protect our web applications against NoSQL Injection

According with OWASP Top 10 - 2017, the most frequent vulnerability in the last year was the A1:2017-Injection, which refers to the injection of a malicious code at a certain point in a vulnerable system, the most known injection is SQL Injection, through URLs, forms we can send malicious queries to the victim's database.

Nowadays, it is common to find systems that have an API to authenticate users and that use a non-relational database to store this information, a much used bank is Mongo.

In the example below, we used the NeDB bank which has a syntax very similar of Mongo.

controller

exports.login = async (req, reply) => {
    try {
        let { user, pass } = req.body

        let result = await findOne({user, pass})

        return reply.code(200).send(result)
    } catch (e) {
        return reply.code(500).send({ success: false, result: 'user/pass not found' })
    }
}
Enter fullscreen mode Exit fullscreen mode

db.findOne

async function findOne(query) {
    return new Promise((resolve, reject) => {
        db.findOne(query, (err, result) => {
            if (err) return reject(err)

            resolve({ success: true, result })
        })
    })
}
Enter fullscreen mode Exit fullscreen mode

The login was made because the object we passed to findOne was a valid object, that is, both user and pass have values that actually exist in the database.

At the beginning of this post I commented on SQL Injection, but have you heard of NoSQL Injection? Not? Okay, you'll understand what this is, see the next function:


db.findOne(query, (err, result) => {
    if (err) return reject(err)

    resolve({ success: true, result })
})
Enter fullscreen mode Exit fullscreen mode

Basically what this function does is a check in the database to know if there is any record with the values we passed to user && pass, note that I used the logical operator && (and ).

This does not make you think that if we pass at least the valid user and instead of the pass inform another validation that returns TRUE, will the function work?

Both Mongo and NeDB have filters that can be used in the queries in the database, for example the $gt, it is the equivalent of the relational operator ">". Let's do a query using this filter instead of the password.

That is, we made a query in the database asking if it has a record with user "wubba" and that the value of pass is greater than "nothing", if there is a user with that name, of course the password will be greater than "nothing".

If we pass the same object {"$ gt": ""} in user and pass, the bank will return the first record it has!

This shows us that the findOne function is dangerous if we do not create treatments for the values we pass to it, in this case we can validate if the information being informed is not an object.

To fix it we can use the following function

controller

exports.loginProtected = async (req, reply) => {
    try {
        let { user, pass } = req.body
        await isObject({ user, pass })

        let result = await findOne({user, pass})

        return reply.code(200).send(result)
    } catch (e) {
        return reply.code(500).send({ success: false, result: 'user/pass not found' })
    }
}
Enter fullscreen mode Exit fullscreen mode

isObject


async function isObject(params) {
    return new Promise((resolve, reject) => {
        Object.keys(params).forEach((v, i) => {
            if (typeof params[v] === 'object') return reject(false)
        })
        resolve(true)
    })
}
Enter fullscreen mode Exit fullscreen mode

This case was reproduced using NeDB bank, but was also simulated using Mongo and Sails/Waterline, if you find in another bank, comment here to help someone else 😉

Github Project https://github.com/nulldreams/nosql-pentest

Top comments (16)

Collapse
 
jalasem profile image
Abdul-Samii Ajala

This vulnerability only applies to a scenario when passwords are stored as plain texts! that itself is a loophole. Store passwords as hashes, not plain texts as depicted in this article

Collapse
 
aksel profile image
aksel

No it doesn't. This vulnerability affects any where clause.

Collapse
 
tiguchi profile image
Thomas Werner

We are given a concrete scenario here which covers user login. And that described scenario is only susceptible to that NoSQL query injection risk because passwords are stored in plain text in the database, which is a big security no-no and absolutely worth pointing out.

When hashed passwords are stored in the database then we are also forced to hash the user-provided password from the login request. In the given example that effectively defuses any NoSQL query injection risk. Any injected query operator would turn into a garbled mess before being used in a query.

Personally I would be OK with discussing the NoSQL injection vulnerability if the scenario was not based on an already flawed premise. No one should be storing passwords in plain text.

Thread Thread
 
ryan profile image
Ryan • Edited

What if you reversed it? Use the injection vulnerability in the username, and guess from a list of common passwords. If any user is using a common password, it should authenticate you.

But another flaw with using "findone" for authentication like this seems to be that you can't use a salt. You should first select the row (or blob? IDK NoSQL) matching the username, and then compare the hashed+salted password.

Thread Thread
 
tiguchi profile image
Thomas Werner

Good point. I turned a blind eye towards the user name since unsalted hashed passwords are also not good enough security-wise due to rainbow table attacks on leaked password hashes. As you point out passwords should be individually salted, so two hashes of the same password are not the same.

The required lookup of the user record for the password's salt hopefully also makes a NoSQL injection attack against the user name infeasible. I would assume that a proper login implementation only looks at a single result from a query, and does not loop over them until it finds a proper match.

Thread Thread
 
aksel profile image
aksel

One way is to match against email (or username or what have you), and then compare the salted hash to the password - outside of NoSQL-land.

Otherwise, what your query is really saying, is that a user could potentially have multiple accounts under the same name, with different passwords.

But yeah, the scenario the author presented the stuff with is flawed. But the point still comes across: Never trust user input. With SQL, protecting yourself from injections is pretty simple. NoSQL seems a bit more tricky

Thread Thread
 
tiguchi profile image
Thomas Werner

Yes, agreed. And JavaScript makes it a little too easy to mess user input validation up since it's so weakly typed. Maybe NoSQL is not entirely to blame here :-D

I think a JavaScript validation library could have been mentioned in the article too. I'd prefer having some proper error feedback from the server when I'm passing in wonky request payloads (by accident or on purpose).

Collapse
 
tiguchi profile image
Thomas Werner

Exactly :-)

Collapse
 
nulldreams profile image
Igor Souza Martins

Yess, it's true! But the attacker can see your registers if u doesn't validate the inputs ☹

Collapse
 
pbnj profile image
Peter Benjamin (they/them)

Good write up.
Some additional resources for those interested in reading more:

Collapse
 
phlash profile image
Phil Ashby

Good advice from the MongoDB team in their documentation, using the same approach as that to defeat injection in SQL, and more generally across other injection attacks - avoid server-side interpreters:

docs.mongodb.com/manual/faq/fundam...

..for MongoDB either through direct use of BSON queries and separate user-data (equivalent of prepared queries in SQL clients), or by disabling server-side Javascript entirely - consider this first IMO!

Collapse
 
nulldreams profile image
Igor Souza Martins

Niceee

Collapse
 
hanipcode profile image
Muhammad Hanif

I am curious do ORM like mongoose handle those NoSQL injection ?

Collapse
 
nulldreams profile image
Igor Souza Martins

Good question Muhammad, I think, with mongo-sanitize you can fix this

Collapse
 
hanipcode profile image
Muhammad Hanif

Nice library. But I think Security issue prevention should be built-in and not an option or even need to use yet another library we need to install for any ORM or database library.
I am currently not having time to check the mongoose source code though.

Collapse
 
j0nnyzer0 profile image
Jon.Ortiz

Great article! The next generation of penetration.