False Positive from .findOne() using Sequelize

[bleow]

I am getting the following vulnerability flagged:

node_nosqli_injection
Untrusted user input in findOne() function can result in NoSQL Injection.

in the following snippet of my code:

const User = require('#models').user
const Role = require('#models').role
const { validate: uuidValidate } = require('uuid')
const tokenData = res.locals.user

const role = await Role.findOne({
    raw: true,
    include: {
      model: User,
      where: {
        id: uuidValidate(tokenData.id) ? tokenData.id : null
      }
    }
  })

For reference, here is role.js

module.exports = (sequelize, DataTypes) => {
  const Role = sequelize.define(
    'role',
    {
       id: {
       ....
       },
       name: {
       ....
       },
       ...
    },
)

As seen from my code, it uses the .findOne() function from sequelize (docs).

I believe this flag is a false-positive, since:

• Using sequelize functions largely prevents SQL attacks (source 2 3)
• My database is SQL-based, so it should not have noSQL vulnerability attacks flagged out.

Looking at the njsscan source code, this issue was flagged out by njsscan/rules/semantic_grep/database/nosql_find_injection.yaml (link), which does a simple pattern matching for .findOne() and its variants.

Perhaps an extra pattern check should be added for code using/importing sequelize, to prevent false positives arising from this rule?

[ajinabraham]

Thanks for the report and the PR. This should be merged now.

[maxalmonte14]

I've been into a rabbit hole since GitLab's pipeline started failing last week at work, it seems this is the cause. We are using TypeORM (which also exposes a findOne method) and Postgres, so there's no way a "NoSQL" injection attack is possible, same as with Sequelize.

@ajinabraham I really don't support jumping into a issue and demanding stuff from a maintainer but is it possible to look into this? I'm not familiar with the library (it was already in place when I started working on the project) but I'd gladly take a look and see if I can implement a similar fix so people using TypeORM and njsscan don't run into this.

Thanks in advance.

[ajinabraham]

If you can share a code snippet with imports and such triggering this false positive, I can add it to the ruleset.

[maxalmonte14]

The following code should trigger the false positive, I tested it using the rule in Semgrep and it works.

import { Repository } from 'typeorm';

export class UserService {
 constructor(
   private userRepository: Repository<User>,
 ) {}

 public async findUser(userIn: UserIn): Promise<User> {
   return this.userRepository.findOne({
     where: {
       id: userIn.id
     },
   });
 }
}

Again, I'm not familiar with the way rules are defined but looking at the fix for Sequelize I guess something along the lines of the following should work, but I'm not sure:

  - pattern-not-inside: |
      $TYPEORM = require('typeorm')
      ...
      $TYPEORM(...)
      ...
  - pattern-not-inside: |
      import $TYPEORM from 'typeorm'
      ...
      $TYPEORM(...)
      ...