Skip to content

Commit 557bd7f

Browse files
authored
fix: prevent object expansion in placeholders after SET clause (#5)
1 parent 968d07e commit 557bd7f

4 files changed

Lines changed: 138 additions & 3 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ deno add npm:sql-escaper
3030

3131
> [!NOTE]
3232
>
33-
> 🔐 **SQL Escaper** fixes a [**SQL Injection vulnerability**](https://flattsecurity.medium.com/finding-an-unseen-sql-injection-by-bypassing-escape-functions-in-mysqljs-mysql-90b27f6542b4) discovered in 2022 in the original [**sqlstring**](https://github.com/mysqljs/sqlstring), where objects passed as values could be expanded into SQL fragments, potentially allowing attackers to manipulate query structure. See [sidorares/node-mysql2#4051](https://github.com/sidorares/node-mysql2/issues/4051) for details.
33+
> 🔐 **SQL Escaper** fixes a potential [**SQL Injection vulnerability**](https://flattsecurity.medium.com/finding-an-unseen-sql-injection-by-bypassing-escape-functions-in-mysqljs-mysql-90b27f6542b4) discovered in 2022 in the original [**sqlstring**](https://github.com/mysqljs/sqlstring), where objects passed as values could be expanded into SQL fragments, potentially allowing attackers to manipulate query structure. See [sidorares/node-mysql2#4051](https://github.com/sidorares/node-mysql2/issues/4051) for details.
3434
3535
---
3636

src/index.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -432,9 +432,10 @@ export const format = (
432432
!Buffer.isBuffer(currentValue) &&
433433
!isDate(currentValue) &&
434434
isRecord(currentValue)
435-
)
435+
) {
436436
escapedValue = objectToValues(currentValue, timezone);
437-
else escapedValue = escape(currentValue, stringifyObjects, timezone);
437+
setIndex = -1;
438+
} else escapedValue = escape(currentValue, stringifyObjects, timezone);
438439
} else escapedValue = escape(currentValue, stringifyObjects, timezone);
439440

440441
result += sql.slice(chunkIndex, placeholderPosition);

test/stringify-objects-as-false.test.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,3 +135,70 @@ describe('SELECT and INSERT with Date parameter', () => {
135135
);
136136
});
137137
});
138+
139+
describe('Object placeholder after SET but outside SET clause', () => {
140+
it('should not expand object in WHERE clause after UPDATE SET', () => {
141+
const query = format('UPDATE users SET ? WHERE id = ?', [
142+
{ name: 'foo' },
143+
{ id: 1 },
144+
]);
145+
146+
assert.strictEqual(
147+
query,
148+
"UPDATE users SET `name` = 'foo' WHERE id = '[object Object]'"
149+
);
150+
});
151+
152+
it('should not expand object in WHERE with multiple conditions after SET', () => {
153+
const query = format('UPDATE users SET ? WHERE id = ? AND role = ?', [
154+
{ name: 'bar' },
155+
{ id: 1 },
156+
'admin',
157+
]);
158+
159+
assert.strictEqual(
160+
query,
161+
"UPDATE users SET `name` = 'bar' WHERE id = '[object Object]' AND role = 'admin'"
162+
);
163+
});
164+
165+
it('should not expand object in WHERE after multiline UPDATE SET', () => {
166+
const query = format(
167+
`UPDATE users
168+
SET ?
169+
WHERE status = ?`,
170+
[{ name: 'foo', email: 'bar@test.com' }, { status: 'active' }]
171+
);
172+
173+
assert.strictEqual(
174+
query,
175+
`UPDATE users
176+
SET \`name\` = 'foo', \`email\` = 'bar@test.com'
177+
WHERE status = '[object Object]'`
178+
);
179+
});
180+
181+
it('should not expand object in subquery after SET', () => {
182+
const query = format(
183+
'UPDATE users SET ? WHERE id IN (SELECT user_id FROM roles WHERE role = ?)',
184+
[{ active: true }, { role: 'admin' }]
185+
);
186+
187+
assert.strictEqual(
188+
query,
189+
"UPDATE users SET `active` = true WHERE id IN (SELECT user_id FROM roles WHERE role = '[object Object]')"
190+
);
191+
});
192+
193+
it('should not expand object in ORDER BY/LIMIT after SET', () => {
194+
const query = format(
195+
'UPDATE users SET ? WHERE active = ? ORDER BY id LIMIT ?',
196+
[{ name: 'test' }, { active: true }, 10]
197+
);
198+
199+
assert.strictEqual(
200+
query,
201+
"UPDATE users SET `name` = 'test' WHERE active = '[object Object]' ORDER BY id LIMIT 10"
202+
);
203+
});
204+
});

test/stringify-objects-as-true.test.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,3 +129,70 @@ describe('SELECT and INSERT with Date parameter', () => {
129129
);
130130
});
131131
});
132+
133+
describe('Object placeholder after SET but outside SET clause', () => {
134+
it('should stringify objects in both SET and WHERE clauses', () => {
135+
const query = format('UPDATE users SET ? WHERE id = ?', [
136+
{ name: 'foo' },
137+
{ id: 1 },
138+
]);
139+
140+
assert.strictEqual(
141+
query,
142+
"UPDATE users SET '[object Object]' WHERE id = '[object Object]'"
143+
);
144+
});
145+
146+
it('should stringify objects in WHERE with multiple conditions after SET', () => {
147+
const query = format('UPDATE users SET ? WHERE id = ? AND role = ?', [
148+
{ name: 'bar' },
149+
{ id: 1 },
150+
'admin',
151+
]);
152+
153+
assert.strictEqual(
154+
query,
155+
"UPDATE users SET '[object Object]' WHERE id = '[object Object]' AND role = 'admin'"
156+
);
157+
});
158+
159+
it('should stringify objects in WHERE after multiline UPDATE SET', () => {
160+
const query = format(
161+
`UPDATE users
162+
SET ?
163+
WHERE status = ?`,
164+
[{ name: 'foo', email: 'bar@test.com' }, { status: 'active' }]
165+
);
166+
167+
assert.strictEqual(
168+
query,
169+
`UPDATE users
170+
SET '[object Object]'
171+
WHERE status = '[object Object]'`
172+
);
173+
});
174+
175+
it('should stringify object in subquery after SET', () => {
176+
const query = format(
177+
'UPDATE users SET ? WHERE id IN (SELECT user_id FROM roles WHERE role = ?)',
178+
[{ active: true }, { role: 'admin' }]
179+
);
180+
181+
assert.strictEqual(
182+
query,
183+
"UPDATE users SET '[object Object]' WHERE id IN (SELECT user_id FROM roles WHERE role = '[object Object]')"
184+
);
185+
});
186+
187+
it('should stringify object in WHERE with ORDER BY/LIMIT after SET', () => {
188+
const query = format(
189+
'UPDATE users SET ? WHERE active = ? ORDER BY id LIMIT ?',
190+
[{ name: 'test' }, { active: true }, 10]
191+
);
192+
193+
assert.strictEqual(
194+
query,
195+
"UPDATE users SET '[object Object]' WHERE active = '[object Object]' ORDER BY id LIMIT 10"
196+
);
197+
});
198+
});

0 commit comments

Comments
 (0)