Skip to content
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 86 additions & 0 deletions javascript/ql/test/query-tests/Security/CWE-089/untyped/hana.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
const hana = require('@sap/hana-client');
const express = require('express');

const app = express();
const connectionParams = {};
app.post('/documents/find', (req, res) => {
const conn = hana.createConnection();
conn.connect(connectionParams, (err) => {
let maliciousInput = req.body.data; // $ MISSING: Source
const query = `SELECT * FROM Users WHERE username = '${maliciousInput}'`;
conn.exec(query, (err, rows) => {}); // $ MISSING: Alert
conn.disconnect();
});

conn.connect(connectionParams, (err) => {
const maliciousInput = req.body.data; // $ MISSING: Source
const stmt = conn.prepare(`SELECT * FROM Test WHERE ID = ? AND username = ` + maliciousInput); // $ MISSING: Alert
stmt.exec([maliciousInput], (err, rows) => {}); // maliciousInput is treated as a parameter
conn.disconnect();
});

conn.connect(connectionParams, (err) => {
const maliciousInput = req.body.data; // $ MISSING: Source
var stmt = conn.prepare(`INSERT INTO Customers(ID, NAME) VALUES(?, ?) ` + maliciousInput); // $ MISSING: Alert
stmt.execBatch([[1, maliciousInput], [2, maliciousInput]], function(err, rows) {}); // maliciousInput is treated as a parameter
conn.disconnect();
});

conn.connect(connectionParams, (err) => {
const maliciousInput = req.body.data; // $ MISSING: Source
var stmt = conn.prepare("SELECT * FROM Customers WHERE ID >= ? AND ID < ?" + maliciousInput); // $ MISSING: Alert
stmt.execQuery([100, maliciousInput], function(err, rs) {}); // $ maliciousInput is treated as a parameter
conn.disconnect();
});
});

var hdbext = require('@sap/hdbext');
var express = require('express');
var dbStream = require('@sap/hana-client/extension/Stream');

var app1 = express();
const hanaConfig = {};
app1.use(hdbext.middleware(hanaConfig));

app1.get('/execute-query', function (req, res) {
var client = req.db;
let maliciousInput = req.body.data; // $ MISSING: Source
client.exec('SELECT * FROM DUMMY' + maliciousInput, function (err, rs) {}); // $ MISSING: Alert

dbStream.createProcStatement(client, 'CALL PROC_DUMMY (?, ?, ?, ?, ?)' + maliciousInput, function (err, stmt) { // $ MISSING: Alert
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is technically SQL injection. It's loading a stored procedure, and calling that with some arguments.

An attacker can definitely not do the usual SQL-injection attacks (e.g. inserting " OR 1=1 ").
But it still seems wrong to allow a user to control that string, so I suppose it's fine.
Did you have any thoughts on this kind of sink?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The concern I had was that, while typical SQL injection might not be possible, there's still a risk that users could invoke procedures they shouldn’t have access to or perform other unauthorized actions. 🤔

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine to err on the side of generating more alerts rather than fewer alerts for this case.

stmt.exec({ A: maliciousInput, B: 4 }, function (err, params, dummyRows, tablesRows) {}); // maliciousInput is treated as a parameter
});

hdbext.loadProcedure(client, null, 'PROC_DUMMY' + maliciousInput, function(err, sp) { // $ MISSING: Alert
sp(3, maliciousInput, function(err, parameters, dummyRows, tablesRows) {}); // maliciousInput is treated as a parameter
});
});


var hdb = require('hdb');
const async = require('async');

const options = {};
const app2 = express();

app2.post('/documents/find', (req, res) => {
var client = hdb.createClient(options);
let maliciousInput = req.body.data; // $ MISSING: Source

client.connect(function onconnect(err) {
async.series([client.exec.bind(client, "INSERT INTO NUMBERS VALUES (1, 'one')" + maliciousInput)], function (err) {}); // $ MISSING: Alert

client.exec('select * from DUMMY' + maliciousInput, function (err, rows) {}); // $ MISSING: Alert
client.exec('select * from DUMMY' + maliciousInput, options, function(err, rows) {}); // $ MISSING: Alert

client.prepare('select * from DUMMY where DUMMY = ?' + maliciousInput, function (err, statement){ // $ MISSING: Alert
statement.exec([maliciousInput], function (err, rows) {}); // maliciousInput is treated as a parameter
});

client.prepare('call PROC_DUMMY (?, ?, ?, ?, ?)' + maliciousInput, function(err, statement){ // $ MISSING: Alert
statement.exec({A: 3, B: maliciousInput}, function(err, parameters, dummyRows, tableRows) {});
});

client.execute('select A, B from TEST.NUMBERS order by A' + maliciousInput, function(err, rs) {}); // $ MISSING: Alert
});
});