SQL injection attacks security issue mysql

General
,
1

I HAVE ALREADY TRIED PREPARED STATEMENTS and STORED PROCEDURES but nothing seems to work.

I am trying to implement a simple API where users can register on the DB. Everything works fine as expected. However, I noticed that I am vulnerable to injection attacks. For example, if I send ';delete from users;' inside the username, the table users is deleted. I hope that using multiple SQL users inside Mariadb with different privileges will solve the issue, however, before trying that I want to find a solution inside Node-red.

I will write parts of all the codes I have tried. All my codes work but they are still vulnerable. I know that I have to sanitize the input but I do not know exactly how and everything I have tried in that respect does not work. Please help.

This is a prepared statement according to MariaDB.

var query = "SET @s1 = 'INSERT INTO users(username, password_hash) VALUES(?, ?);';" +
"PREPARE stmt1 FROM @s1;" +
"SET @a = '"+ username +"';" +
"SET @b = '" + password + "';" +
"EXECUTE stmt1 USING @a, @b;" +
"DEALLOCATE PREPARE stmt1;";

I made a stored procedure inside the DB, and I called with this.

let query = "CALL newUser('" + username +"', '" + password +"', '" + name + "')";

This would be the prepared statement according to the documentation of the node node-red-node-mysql

var query = "INSERT INTO users(username, password_hash) VALUES(?, ?);";
msg.payload = [username, password];
msg.topic = query;

This one is according to the documentation of node node-red-contrib-mysql-config

msg.topic = 'INSERT INTO users(username, password_hash) VALUES(:user, :pass);';
msg.payload = { user: username, pass: password };

This is the weakest one

let query ="insert into users(username, password_hash) values('" +
username + "', '"+ password + "');"
2

So, all user input MUST ALWAYS BE SANITISED as you've already learned. For SQL injection and some other common attacks, you need to:

  • Limit the input string length to something sensible. For example user ID's are probably never longer the 64 chars, maybe less, certainly not more than 255 characters. Passwords should certainly never be longer than 255 characters (funny aside: I always set my passwords to 63 random characters to test whether systems I use have decent password settings :slight_smile: )

  • Passwords must never be stored. You need to convert them into a hash string as early as possible. This will, of course, convert any ; or other meaningful characters to something safe since the hash string is almost certainly just HEX (I think?).

  • Filter out characters that are meaningful in SQL statements. In other words, use either a change node or a function node to remove ; from any input strings.

Do these things BEFORE you go anywhere near your SQL.

BTW, limiting input string length is often overlooked but is important to stop fuzzing attacks.

Also, make sure that you only use POST and not GET http types for inputs like this. GET would put parameters onto the URL which brings a whole host of other issues.

And, of course, security without HTTPS (TLS encryption) is no security at all.

3

I've not tried it but possible GitHub - pariazar/perfect-express-sanitizer: a complete package to control user input data to prevent Cross Site Scripting (XSS) ,Sql injection and no Sql injection attack can help?

As a bare minimum striping out none alphabetic characters help but reduces security for user passwords - as per @TotallyInformation states - hash ASAP and just store that.

4

To support @TotallyInformation's input

  • Run the password through a hash process (ensure its also hashed before comparison later)
    this should remove anything dodge, given you should output as a hexadeicmal string! and should be done before reaching the query
    node-red-contrib-cryptography (node) - Node-RED

  • Strip any unwanted characters in the username (and any other field you need)

const yikesChars = [';',':','--'] /* Example */
let sanitizedUsername
[...msg.payload.username].forEach(char => { 
	if(!yikesChars.includes(char){
        sanitisedUsername += char
    }
});
msg.payload.username = sanitizedUsername

I'm sure there are cleaner/better ways, but I was bored (and I am S**T) at regex :smile:

However, I always thought using named params was already a protection method?

like you already stated?

var query = "INSERT INTO users(username, password_hash) VALUES(?, ?);";
msg.payload = [username, password];
msg.topic = query;
2 Likes
5

Shouldn't that be S.{2}T ? :crazy_face:

1 Like
6

Buurrrnnn! :joy:

7

It should be used as well. But practicing "Secure by Design" is always a good thing and that will still have you sanitising the inputs before anything else.

1 Like
8

I was ready for that response Julian (but in agreement) :blush:

1 Like
9

Actually, there is a new UK Government initiative and framework that is being burned into all public sector processes called "Secure by Design" so it is rather fresh in my mind :laughing:

1 Like
10

Is that the same government that had secured the electoral role?

2 Likes
11

Which is maybe why they felt the need to be a bit more explicit with public sector about security! :slight_smile:

12

This function helped a lot, thanks @marcus-j-davies I have improved it and will share it in the next comment.

@marcus-j-davies I wish named params worked, but nope, they are useless, at least in node-red.

13

Thanks to @marcus-j-davies I figured out what I could do. The next code sanitizes the string, cleaning special characters and SQL Query words. Feel free to use it and to improve it.

const yikesChars = [';', ':', '--', `'`, '\\', '<', '>', '!', '=', '(', ')'];
function deleteWords(inputString) {
    const sqlKeywords = ['SELECT', 'FROM', 'DELETE', 'UPDATE', 'CREATE', 'INSERT', 'DROP', 'ALTER', 'TRUNCATE'];
    let normalizedInput = inputString.toLowerCase();
    sqlKeywords.forEach(keyword => {
        let regex = new RegExp(`\\b${keyword.toLowerCase()}\\b`, 'gi');
        normalizedInput = normalizedInput.replace(regex, '');
    });
    return normalizedInput;
}

function sanitizeString(inputString, charsToExclude) {
    let sanitizedString = '';
    let chars = inputString.split('');
    for (let i = 0; i < chars.length; i++) {
        let substr = chars[i] + (chars[i + 1] || '');
        if (charsToExclude.includes(substr)) {
            i++;
            continue;
        }
        if (!charsToExclude.includes(chars[i])) {
            sanitizedString += chars[i];
        }
    }
    sanitizedString = deleteWords(sanitizedString);
    return sanitizedString;
}

//using it 
username = sanitizeString(username, yikesChars);
2 Likes
14

I am having trouble replicating this.
With no stored procedure defined in the database, my code

msg.payload = {"username": "Donald Trump; delete from users;"}
msg.topic = "insert into users (username) values (:username)"
return msg;

gives me these records

> select * from users;
+----+----------------------------------+---------------+
| id | username                         | password_hash |
+----+----------------------------------+---------------+
|  1 | Joe Biden                        | NULL          |
|  2 | Donald Trump; delete from users; | NULL          |
+----+----------------------------------+---------------+
2 rows in set (0.001 sec)

What am I doing wrong?

15

I thought it was obvious, the only way to remove Trump seems to be by the ballot box. He seems invulnerable to any form of attack at the present time. (this comment does not express any form of support for any political side, it is simply a joke)

7 Likes
16

I send an http post with the user's header using ';delete from users;' and i take it as my username. I don't know if it can be replicated inside the node. If it were possible to replicated it inside the node then you are missing the '; initials and the ' final, but again, Idk if it can be replicated like that.

17

I am a bit late to the party.
The correct answer is to use placeholders in the SQL assigned to the message topic property and pass values to the message payload; this has already been stated.

Escape Function
Why not use the Node MySQL built-in escape function for those using custom functions?

You can load the MySQL node as a global function in settings and call in the function node.

const mysql = global.get("mysql");

Or dynamically load it in the function node setup tab.
Simple go to the Setup Tab and type msql for the Module name

Screenshot 2024-07-04 at 8.57.33 AM

Once you have loaded it once, it will then be a drop-down option in the future
Screenshot 2024-07-04 at 8.58.23 AM

//start transaction;
let insert = "START TRANSACTION;";

//insert part number
insert += "INSERT INTO vehicle_partnumbers (partnumber,name,ShortDescription,lastmodified) VALUES (" + mysql.escape(data.partnumber) + "," + mysql.escape(data.name) + ",";
insert += mysql.escape(data.shortdescription) + "," + mysql.escape(data.lastmodified) + ") ";
insert += "ON DUPLICATE KEY UPDATE name=" + mysql.escape(data.name) + ", ShortDescription=" + mysql.escape(data.shortdescription) + ", lastmodified=" + mysql.escape(data.lastmodified) + ",";
insert += "partnumber_id=LAST_INSERT_ID(partnumber_id);";

insert += "SET @last_id_in_vehicle_partnumbers = LAST_INSERT_ID();";

Escape ID Function
If you can't trust an SQL identifier (database / table / column name) because it is provided by a user, you should escape it with escapeId()

mysql.escapeId(identifier)

Example

var sorter = 'date';
var sql    = 'SELECT * FROM posts ORDER BY ' + mysql.escapeId(sorter);

It also supports adding qualified identifiers. It will escape both parts.

var sorter = 'date';
var sql    = 'SELECT * FROM posts ORDER BY ' + mysql.escapeId('posts.' + sorter);
// -> SELECT * FROM posts ORDER BY `posts`.`date`e

Alternatively, you can use ?? characters as placeholders for identifiers you would like to have escaped like this:

var userId = 1;
var columns = ['username', 'email'];
var query = connection.query('SELECT ?? FROM ?? WHERE id = ?', [columns, 'users', userId], function (error, results, fields) {
  if (error) throw error;
  // ...
});

console.log(query.sql); // SELECT `username`, `email` FROM `users` WHERE id = 1

Use a single ? for a query value and a double ?? for identifiers.

I have used both methods.

1 Like