fix: SQL injection via Increment operation on nested object field in PostgreSQL ([GHSA-q3vj-96h2-gwvg](GHSA-q3vj-96h2-gwvg)) (#10161)

Author: mtrezza

spec/vulnerabilities.spec.js

@@ -1049,3 +1049,91 @@ describe('(GHSA-3jmq-rrxf-gqrg) Stored XSS via file serving', () => {
     expect(response.headers['x-content-type-options']).toBe('nosniff');
   });
 });
+
+describe('(GHSA-q3vj-96h2-gwvg) SQL Injection via Increment amount on nested Object field', () => {
+  const headers = {
+    'Content-Type': 'application/json',
+    'X-Parse-Application-Id': 'test',
+    'X-Parse-REST-API-Key': 'rest',
+  };
+
+  it('rejects non-number Increment amount on nested object field', async () => {
+    const obj = new Parse.Object('IncrTest');
+    obj.set('stats', { counter: 0 });
+    await obj.save();
+
+    const response = await request({
+      method: 'PUT',
+      url: `http://localhost:8378/1/classes/IncrTest/${obj.id}`,
+      headers,
+      body: JSON.stringify({
+        'stats.counter': { __op: 'Increment', amount: '1' },
+      }),
+    }).catch(e => e);
+
+    expect(response.status).toBe(400);
+    const text = JSON.parse(response.text);
+    expect(text.code).toBe(Parse.Error.INVALID_JSON);
+  });
+
+  it_only_db('postgres')('does not execute injected SQL via Increment amount with pg_sleep', async () => {
+    const obj = new Parse.Object('IncrTest');
+    obj.set('stats', { counter: 0 });
+    await obj.save();
+
+    const start = Date.now();
+    await request({
+      method: 'PUT',
+      url: `http://localhost:8378/1/classes/IncrTest/${obj.id}`,
+      headers,
+      body: JSON.stringify({
+        'stats.counter': { __op: 'Increment', amount: '0+(SELECT 1 FROM pg_sleep(3))' },
+      }),
+    }).catch(() => {});
+    const elapsed = Date.now() - start;
+
+    // If injection succeeded, query would take >= 3 seconds
+    expect(elapsed).toBeLessThan(3000);
+  });
+
+  it_only_db('postgres')('does not execute injected SQL via Increment amount for data exfiltration', async () => {
+    const obj = new Parse.Object('IncrTest');
+    obj.set('stats', { counter: 0 });
+    await obj.save();
+
+    await request({
+      method: 'PUT',
+      url: `http://localhost:8378/1/classes/IncrTest/${obj.id}`,
+      headers,
+      body: JSON.stringify({
+        'stats.counter': {
+          __op: 'Increment',
+          amount: '0+(SELECT ascii(substr(current_database(),1,1)))',
+        },
+      }),
+    }).catch(() => {});
+
+    // Verify counter was not modified by injected SQL
+    const verify = await new Parse.Query('IncrTest').get(obj.id);
+    expect(verify.get('stats').counter).toBe(0);
+  });
+
+  it('allows valid numeric Increment on nested object field', async () => {
+    const obj = new Parse.Object('IncrTest');
+    obj.set('stats', { counter: 5 });
+    await obj.save();
+
+    const response = await request({
+      method: 'PUT',
+      url: `http://localhost:8378/1/classes/IncrTest/${obj.id}`,
+      headers,
+      body: JSON.stringify({
+        'stats.counter': { __op: 'Increment', amount: 3 },
+      }),
+    });
+
+    expect(response.status).toBe(200);
+    const verify = await new Parse.Query('IncrTest').get(obj.id);
+    expect(verify.get('stats').counter).toBe(8);
+  });
+});

src/Adapters/Storage/Postgres/PostgresStorageAdapter.js

@@ -1735,13 +1735,19 @@ export class PostgresStorageAdapter implements StorageAdapter {
           .map(k => k.split('.')[1]);
 
         let incrementPatterns = '';
+        const incrementValues = [];
         if (keysToIncrement.length > 0) {
           incrementPatterns =
             ' || ' +
             keysToIncrement
               .map(c => {
                 const amount = fieldValue[c].amount;
-                return `CONCAT('{"${c}":', COALESCE($${index}:name->>'${c}','0')::int + ${amount}, '}')::jsonb`;
+                if (typeof amount !== 'number') {
+                  throw new Parse.Error(Parse.Error.INVALID_JSON, 'incrementing must provide a number');
+                }
+                incrementValues.push(amount);
+                const amountIndex = index + incrementValues.length;
+                return `CONCAT('{"${c}":', COALESCE($${index}:name->>'${c}','0')::int + $${amountIndex}, '}')::jsonb`;
               })
               .join(' || ');
           // Strip the keys
@@ -1764,7 +1770,7 @@ export class PostgresStorageAdapter implements StorageAdapter {
           .map(k => k.split('.')[1]);
 
         const deletePatterns = keysToDelete.reduce((p: string, c: string, i: number) => {
-          return p + ` - '$${index + 1 + i}:value'`;
+          return p + ` - '$${index + 1 + incrementValues.length + i}:value'`;
         }, '');
         // Override Object
         let updateObject = "'{}'::jsonb";
@@ -1774,11 +1780,11 @@ export class PostgresStorageAdapter implements StorageAdapter {
           updateObject = `COALESCE($${index}:name, '{}'::jsonb)`;
         }
         updatePatterns.push(
-          `$${index}:name = (${updateObject} ${deletePatterns} ${incrementPatterns} || $${index + 1 + keysToDelete.length
+          `$${index}:name = (${updateObject} ${deletePatterns} ${incrementPatterns} || $${index + 1 + incrementValues.length + keysToDelete.length
           }::jsonb )`
         );
-        values.push(fieldName, ...keysToDelete, JSON.stringify(fieldValue));
-        index += 2 + keysToDelete.length;
+        values.push(fieldName, ...incrementValues, ...keysToDelete, JSON.stringify(fieldValue));
+        index += 2 + incrementValues.length + keysToDelete.length;
       } else if (
         Array.isArray(fieldValue) &&
         schema.fields[fieldName] &&