Skip to content

Commit 4fab072

Browse files
committed
fix
1 parent d306b02 commit 4fab072

File tree

2 files changed

+52
-2
lines changed

2 files changed

+52
-2
lines changed

spec/vulnerabilities.spec.js

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1352,3 +1352,50 @@ 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+
});
1382+
1383+
it_only_db('postgres')('allows valid Increment on nested object field with normal sub-key', async () => {
1384+
const obj = new Parse.Object('SubKeyTest');
1385+
obj.set('stats', { counter: 5 });
1386+
await obj.save();
1387+
1388+
const response = await request({
1389+
method: 'PUT',
1390+
url: `http://localhost:8378/1/classes/SubKeyTest/${obj.id}`,
1391+
headers,
1392+
body: JSON.stringify({
1393+
'stats.counter': { __op: 'Increment', amount: 2 },
1394+
}),
1395+
});
1396+
1397+
expect(response.status).toBe(200);
1398+
const verify = await new Parse.Query('SubKeyTest').get(obj.id);
1399+
expect(verify.get('stats').counter).toBe(7);
1400+
});
1401+
});

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)