Fix Unsafe SQL Queries in Knex Example Files#446
Conversation
| try { | ||
| // NOTE: MySQL syntax FOR UPDATE for read lock on counter stats in row | ||
| const row = await trx('rate_limits') | ||
| .whereRaw('route = ? AND source = ? FOR UPDATE', [cond.route || '', cond.source]) // Create read lock |
There was a problem hiding this comment.
SQL-injection protected here because this uses parameter bindings.
| // Optimization - no need to UPDATE if max has been reached. | ||
| if (d.count < max) { | ||
| await trx | ||
| .raw('UPDATE rate_limits SET count = ? WHERE route = ? AND source = ?', [d.count + 1, cond.route, key]) |
There was a problem hiding this comment.
SQL-injection protected here because this uses parameter bindings.
| process.nextTick(cb, null, { current: d.count + 1, ttl: d.ttl }) | ||
| } else { | ||
| await trx | ||
| .raw('INSERT INTO rate_limits(route, source, count, ttl) VALUES(?,?,1,?) ON DUPLICATE KEY UPDATE count = 1, ttl = ?', [cond.route, key, d?.ttl || ttl, ttl]) |
There was a problem hiding this comment.
SQL-injection protected here because this uses parameter bindings.
| }) | ||
| } else { | ||
| trx | ||
| .raw(`INSERT INTO RateLimits(Route, Source, Count, TTL) VALUES('${this.route}', '${key}',1,${d.TTL || ttl}) ON CONFLICT(Route, Source) DO UPDATE SET Count=Count+1,TTL=${ttl}`) |
There was a problem hiding this comment.
I agree this query is generally unsafe because it interpolates values directly into the SQL string.
That said, the values appear to come from plugin/user configuration rather than directly from a network client. So much less critical than the PR description suggests.
There was a problem hiding this comment.
Ok for changing this, let's keep the other queries as they are
|
I have reverted the changes to |
2 participants
The Knex example files (
example/example-knex-mysql.jsandexample/example-knex.js) contained unsafe raw SQL queries that directly interpolated user-supplied values (this.routeandkey) into SQL strings, making them vulnerable to SQL injection. This change replaces those raw queries with proper parameterized queries using Knex's binding syntax, which prevents injection attacks and improves safety. The changes are necessary because the examples are distributed as part of the package and could be used as reference by users.