Skip to content

Commit 169d692

Browse files
authored
fix: SQL Injection via dot-notation sub-key name in Increment operation on PostgreSQL ([GHSA-gqpp-xgvh-9h7h](GHSA-gqpp-xgvh-9h7h)) (#10165)
1 parent d306b02 commit 169d692

File tree

2 files changed

+107
-2
lines changed

2 files changed

+107
-2
lines changed

spec/vulnerabilities.spec.js

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1352,3 +1352,105 @@ describe('(GHSA-q3vj-96h2-gwvg) SQL Injection via Increment amount on nested Obj
13521352
expect(verify.get('stats').counter).toBe(8);
13531353
});
13541354
});
1355+
1356+
describe('(GHSA-gqpp-xgvh-9h7h) SQL Injection via dot-notation sub-key name in Increment operation', () => {
1357+
const headers = {
1358+
'Content-Type': 'application/json',
1359+
'X-Parse-Application-Id': 'test',
1360+
'X-Parse-REST-API-Key': 'rest',
1361+
};
1362+
1363+
it_only_db('postgres')('does not execute injected SQL via single quote in sub-key name', async () => {
1364+
const obj = new Parse.Object('SubKeyTest');
1365+
obj.set('stats', { counter: 0 });
1366+
await obj.save();
1367+
1368+
const start = Date.now();
1369+
await request({
1370+
method: 'PUT',
1371+
url: `http://localhost:8378/1/classes/SubKeyTest/${obj.id}`,
1372+
headers,
1373+
body: JSON.stringify({
1374+
"stats.x' || (SELECT pg_sleep(3))::text || '": { __op: 'Increment', amount: 1 },
1375+
}),
1376+
}).catch(() => {});
1377+
const elapsed = Date.now() - start;
1378+
1379+
// If injection succeeded, query would take >= 3 seconds
1380+
expect(elapsed).toBeLessThan(3000);
1381+
// The escaped payload becomes a harmless literal key; original data is untouched
1382+
const verify = await new Parse.Query('SubKeyTest').get(obj.id);
1383+
expect(verify.get('stats').counter).toBe(0);
1384+
});
1385+
1386+
it_only_db('postgres')('does not execute injected SQL via double quote in sub-key name', async () => {
1387+
const obj = new Parse.Object('SubKeyTest');
1388+
obj.set('stats', { counter: 0 });
1389+
await obj.save();
1390+
1391+
const start = Date.now();
1392+
await request({
1393+
method: 'PUT',
1394+
url: `http://localhost:8378/1/classes/SubKeyTest/${obj.id}`,
1395+
headers,
1396+
body: JSON.stringify({
1397+
'stats.x" || (SELECT pg_sleep(3))::text || "': { __op: 'Increment', amount: 1 },
1398+
}),
1399+
}).catch(() => {});
1400+
const elapsed = Date.now() - start;
1401+
1402+
// Double quotes break JSON structure inside the CONCAT, producing invalid JSONB.
1403+
// This causes a database error, NOT SQL injection. If injection succeeded,
1404+
// the query would take >= 3 seconds due to pg_sleep.
1405+
expect(elapsed).toBeLessThan(3000);
1406+
// Invalid JSONB cast fails the UPDATE, so the row is not modified
1407+
const verify = await new Parse.Query('SubKeyTest').get(obj.id);
1408+
expect(verify.get('stats')).toEqual({ counter: 0 });
1409+
});
1410+
1411+
it_only_db('postgres')('does not execute injected SQL via double quote crafted as valid JSONB in sub-key name', async () => {
1412+
const obj = new Parse.Object('SubKeyTest');
1413+
obj.set('stats', { counter: 0 });
1414+
await obj.save();
1415+
1416+
// This payload uses double quotes to craft a sub-key that produces valid JSONB
1417+
// (e.g. '{"x":0,"evil":1}') instead of breaking JSON structure. Even so, both
1418+
// interpolation sites are inside single-quoted SQL strings, so double quotes
1419+
// cannot escape the SQL context — no arbitrary SQL execution is possible.
1420+
const start = Date.now();
1421+
await request({
1422+
method: 'PUT',
1423+
url: `http://localhost:8378/1/classes/SubKeyTest/${obj.id}`,
1424+
headers,
1425+
body: JSON.stringify({
1426+
'stats.x":0,"pg_sleep(3)': { __op: 'Increment', amount: 1 },
1427+
}),
1428+
}).catch(() => {});
1429+
const elapsed = Date.now() - start;
1430+
1431+
expect(elapsed).toBeLessThan(3000);
1432+
// Double quotes craft valid JSONB with extra keys, but no SQL injection occurs;
1433+
// original counter is untouched
1434+
const verify = await new Parse.Query('SubKeyTest').get(obj.id);
1435+
expect(verify.get('stats').counter).toBe(0);
1436+
});
1437+
1438+
it_only_db('postgres')('allows valid Increment on nested object field with normal sub-key', async () => {
1439+
const obj = new Parse.Object('SubKeyTest');
1440+
obj.set('stats', { counter: 5 });
1441+
await obj.save();
1442+
1443+
const response = await request({
1444+
method: 'PUT',
1445+
url: `http://localhost:8378/1/classes/SubKeyTest/${obj.id}`,
1446+
headers,
1447+
body: JSON.stringify({
1448+
'stats.counter': { __op: 'Increment', amount: 2 },
1449+
}),
1450+
});
1451+
1452+
expect(response.status).toBe(200);
1453+
const verify = await new Parse.Query('SubKeyTest').get(obj.id);
1454+
expect(verify.get('stats').counter).toBe(7);
1455+
});
1456+
});

src/Adapters/Storage/Postgres/PostgresStorageAdapter.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,8 @@ const handleDotFields = object => {
208208
return object;
209209
};
210210

211+
const escapeSqlString = value => value.replace(/'/g, "''");
212+
211213
const transformDotFieldToComponents = fieldName => {
212214
return fieldName.split('.').map((cmpt, index) => {
213215
if (index === 0) {
@@ -216,7 +218,7 @@ const transformDotFieldToComponents = fieldName => {
216218
if (isArrayIndex(cmpt)) {
217219
return Number(cmpt);
218220
} else {
219-
return `'${cmpt.replace(/'/g, "''")}'`;
221+
return `'${escapeSqlString(cmpt)}'`;
220222
}
221223
});
222224
};
@@ -1747,7 +1749,8 @@ export class PostgresStorageAdapter implements StorageAdapter {
17471749
}
17481750
incrementValues.push(amount);
17491751
const amountIndex = index + incrementValues.length;
1750-
return `CONCAT('{"${c}":', COALESCE($${index}:name->>'${c}','0')::int + $${amountIndex}, '}')::jsonb`;
1752+
const safeName = escapeSqlString(c);
1753+
return `CONCAT('{"${safeName}":', COALESCE($${index}:name->>'${safeName}','0')::int + $${amountIndex}, '}')::jsonb`;
17511754
})
17521755
.join(' || ');
17531756
// Strip the keys

0 commit comments

Comments
 (0)