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

[mtrezza]

Pull Request

• Report security issues confidentially.
• Any contribution is under this license.

Issue

Approach

SQL injection via Increment operation on nested object field in PostgreSQL (GHSA-q3vj-96h2-gwvg)

• Add tests
• Add changes to documentation (guides, repository pages, code comments)
• Add security check
• Add new Parse Error codes to Parse JS SDK

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • Added validation to require numeric values for increment operations on nested object fields; non-numeric amounts now return an error.
  • Fixed SQL injection vulnerability in increment operations on nested object fields.

[parse-github-assistant]

🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review.

Note

Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect.

Caution

Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement.

[parseplatformorg]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds input validation and test coverage for SQL injection vulnerability (GHSA-q3vj-96h2-gwvg) affecting Increment operations on nested object fields. The Postgres adapter now validates that increment amounts are numeric values, and a comprehensive test suite verifies the fix prevents SQL injection attacks and data exfiltration attempts.

Changes

Cohort / File(s)	Summary
Vulnerability Test Suite spec/vulnerabilities.spec.js	Adds four tests validating Increment operation security on nested object fields: rejection of non-numeric amounts (400 error), prevention of SQL injection via pg_sleep(3), prevention of data exfiltration via database name extraction, and verification of valid numeric increments.
Postgres Adapter Input Validation src/Adapters/Storage/Postgres/PostgresStorageAdapter.js	Introduces input validation for increment operations requiring numeric amounts (throws INVALID_JSON if not a number). Updates SQL fragment generation and value array ordering to properly handle multiple increments before deletions on nested object fields.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check	✅ Passed	The title directly and specifically addresses the main change: fixing a SQL injection vulnerability (GHSA-q3vj-96h2-gwvg) in the Increment operation on nested object fields in PostgreSQL.
Description check	✅ Passed	The PR description follows the template structure with Issue and Approach sections. The Approach clearly describes the security fix, and the Tasks section shows one checkbox marked as completed (Add tests).

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

spec/vulnerabilities.spec.js (2)

1079-1118: Assert the INVALID_JSON rejection in the Postgres probes.

These two tests currently pass on any fast failure or silent no-op. Since the adapter now rejects string amounts explicitly, please assert the 400/code/message here too; otherwise a different parser or DB error would still look green.

Suggested tightening

-    await request({
+    const response = 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(() => {});
+    }).catch(e => e);
     const elapsed = Date.now() - start;
 
+    expect(response.status).toBe(400);
+    const text = JSON.parse(response.text);
+    expect(text.code).toBe(Parse.Error.INVALID_JSON);
+    expect(text.error).toBe('incrementing must provide a number');
     expect(elapsed).toBeLessThan(3000);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@spec/vulnerabilities.spec.js` around lines 1079 - 1118, Update the two tests
named "does not execute injected SQL via Increment amount with pg_sleep" and
"does not execute injected SQL via Increment amount for data exfiltration" to
assert that the adapter rejects string amounts with the expected
INVALID_JSON/400 response instead of just timing or silent no-op: call request
and capture the response/error, then expect the HTTP status to be 400 (or the
returned error.code/message to equal INVALID_JSON) before proceeding; keep the
existing verification that the Parse.Object ('IncrTest') counter remains
unchanged (the save/get logic using Parse.Query and the obj.id) but replace the
current catch-and-ignore pattern with explicit assertions on the error payload
from the request call.

---

1053-1139: Add one multi-field nested update regression.

The adapter change reworked placeholder ordering for incrementValues and delete keys, but this suite only exercises a single nested increment. One request that updates stats.a and stats.b together, and ideally one that mixes nested increment + nested delete on the same object, would cover the new indexing logic much more directly.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@spec/vulnerabilities.spec.js` around lines 1053 - 1139, Add a new multi-field
nested update test that updates two nested keys in the same object in a single
PUT (e.g., 'stats.a' and 'stats.b' both with { __op: 'Increment', amount:
<number> }) and assert both fields are incremented correctly, and add another
test that mixes a nested Increment and a nested Delete on the same parent object
(e.g., 'stats.x': { __op: 'Increment', amount: 1 } and 'stats.y': { __op:
'Delete' }) and verify after the request that the increment applied and the
deleted key is removed; base both tests on the existing patterns used in the
file (construct Parse.Object('IncrTest'), save initial stats, send PUT to
/1/classes/IncrTest/{id}, then fetch with new Parse.Query('IncrTest').get(id)
and assert final state) so the adapter's multi-field placeholder ordering and
delete-key handling are exercised.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/Adapters/Storage/Postgres/PostgresStorageAdapter.js`:
- Around line 1743-1750: The code in updateObjectsByQuery uses the loop variable
c from originalUpdate and directly interpolates it into the SQL fragment
(`->>'${c}'` and `CONCAT('{"${c}":'...)`), which can break SQL/JSON for keys
containing quotes or special chars; instead push the key c into the query
parameters and build the JSON using parameterized functions: add c to the
parameter array (alongside incrementAmounts in incrementValues), replace the
interpolated CONCAT fragment with a parameterized jsonb_build_object call that
uses name->>$keyParam for safe key lookup and adds $amountParam (e.g.
jsonb_build_object($keyParam, (COALESCE(name->>$keyParam,'0')::int +
$amountParam)) ), and adjust the indices (index, amountIndex) so the key
parameter is in the correct position; do not interpolate c directly and ensure
originalUpdate/incrementValues and the values array reflect the new parameter
ordering.

---

Nitpick comments:
In `@spec/vulnerabilities.spec.js`:
- Around line 1079-1118: Update the two tests named "does not execute injected
SQL via Increment amount with pg_sleep" and "does not execute injected SQL via
Increment amount for data exfiltration" to assert that the adapter rejects
string amounts with the expected INVALID_JSON/400 response instead of just
timing or silent no-op: call request and capture the response/error, then expect
the HTTP status to be 400 (or the returned error.code/message to equal
INVALID_JSON) before proceeding; keep the existing verification that the
Parse.Object ('IncrTest') counter remains unchanged (the save/get logic using
Parse.Query and the obj.id) but replace the current catch-and-ignore pattern
with explicit assertions on the error payload from the request call.
- Around line 1053-1139: Add a new multi-field nested update test that updates
two nested keys in the same object in a single PUT (e.g., 'stats.a' and
'stats.b' both with { __op: 'Increment', amount: <number> }) and assert both
fields are incremented correctly, and add another test that mixes a nested
Increment and a nested Delete on the same parent object (e.g., 'stats.x': {
__op: 'Increment', amount: 1 } and 'stats.y': { __op: 'Delete' }) and verify
after the request that the increment applied and the deleted key is removed;
base both tests on the existing patterns used in the file (construct
Parse.Object('IncrTest'), save initial stats, send PUT to
/1/classes/IncrTest/{id}, then fetch with new Parse.Query('IncrTest').get(id)
and assert final state) so the adapter's multi-field placeholder ordering and
delete-key handling are exercised.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4fad3927-c921-4072-8c4e-ebfeda483d29

📥 Commits

Reviewing files that changed from the base of the PR and between a6c0926 and b357303.

📒 Files selected for processing (2)

• spec/vulnerabilities.spec.js
• src/Adapters/Storage/Postgres/PostgresStorageAdapter.js

[parseplatformorg]

🎉 This change has been released in version 9.6.0-alpha.3

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.17%. Comparing base (ea538a4) to head (b357303).
⚠️ Report is 123 commits behind head on alpha.

Additional details and impacted files

@@            Coverage Diff             @@
##            alpha   #10161      +/-   ##
==========================================
- Coverage   92.58%   92.17%   -0.41%     
==========================================
  Files         192      192              
  Lines       16207    16212       +5     
  Branches      183      183              
==========================================
- Hits        15005    14944      -61     
- Misses       1190     1252      +62     
- Partials       12       16       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[parseplatformorg]

🎉 This change has been released in version 9.6.0