Skip to content

fix: Pre-authentication denial of service via client version header regex backtracking (GHSA-38m6-82c8-4xfm)#10463

Merged
mtrezza merged 2 commits into
parse-community:alphafrom
mtrezza:fix/GHSA-38m6-82c8-4xfm-9.x
May 17, 2026
Merged

fix: Pre-authentication denial of service via client version header regex backtracking (GHSA-38m6-82c8-4xfm)#10463
mtrezza merged 2 commits into
parse-community:alphafrom
mtrezza:fix/GHSA-38m6-82c8-4xfm-9.x

Conversation

@mtrezza
Copy link
Copy Markdown
Member

@mtrezza mtrezza commented May 17, 2026

Issue

Pre-authentication denial of service via client version header regex backtracking (GHSA-38m6-82c8-4xfm)

Tasks

  • Add tests
  • Add changes to documentation (guides, repository pages, in-code descriptions)

@parse-github-assistant
Copy link
Copy Markdown

parse-github-assistant Bot commented May 17, 2026

🚀 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.

Tip

  • Keep pull requests small. Large PRs will be rejected. Break complex features into smaller, incremental PRs.
  • Use Test Driven Development. Write failing tests before implementing functionality. Ensure tests pass.
  • Group code into logical blocks. Add a short comment before each block to explain its purpose.
  • We offer conceptual guidance. Coding is up to you. PRs must be merge-ready for human review.
  • Our review focuses on concept, not quality. PRs with code issues will be rejected. Use an AI agent.
  • Human review time is precious. Avoid review ping-pong. Inspect and test your AI-generated code.

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. Our CI and AI review are safeguards, not development tools. If many issues are flagged, rethink your development approach. Invest more effort in planning and design rather than using review cycles to fix low-quality code.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 17, 2026

📝 Walkthrough

Walkthrough

This PR removes end-to-end client version and SDK parsing by deleting the ClientSDK module, stopping middleware extraction of version headers/body fields, removing clientSDK parameter forwarding through REST and GraphQL layers, and changing delete-operation response handling to be unconditional rather than capability-gated.

Changes

ClientSDK version parsing removal

Layer / File(s) Summary
Middleware client version removal and ReDoS prevention tests
src/middlewares.js, spec/Middlewares.spec.js, spec/vulnerabilities.spec.js
Middleware stops extracting X-Parse-Client-Version header and _ClientVersion body field; _ClientVersion is deleted from req.body without validation, leaving req.info.clientVersion and req.info.clientSDK unset. Tests updated to remove clientVersion expectations and add vulnerability tests that assert no regex-driven performance blowups and proper stripping.
REST core layer signature and delete-operation updates
src/rest.js, src/RestQuery.js, src/RestWrite.js
Exported find/get/create/update remove clientSDK parameter; RestQuery and _UnsafeRestQuery constructors no longer accept or store clientSDK. RestWrite drops the ClientSDK import and preserves delete-operation values in responses unconditionally instead of gating on client capability.
Router REST call sites
src/Routers/ClassesRouter.js, src/Routers/SessionsRouter.js, src/Routers/UsersRouter.js, src/Routers/AggregateRouter.js, src/Routers/AudiencesRouter.js, src/Routers/InstallationsRouter.js, src/Routers/IAPValidationRouter.js
Router handlers update downstream rest.find/rest.get/rest.create/rest.update calls to omit req.info.clientSDK and pass only req.info.context as the final context parameter.
GraphQL helper REST call sites
src/GraphQL/helpers/objectsMutations.js, src/GraphQL/helpers/objectsQueries.js, src/GraphQL/loaders/usersQueries.js
GraphQL helpers remove info.clientSDK from rest.* call parameter lists and forward only info.context as the final info argument.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • parse-community/parse-server#10406: Both PRs modify src/Routers/SessionsRouter.js—specifically handleMe's rest.find/rest.get call paths—with overlapping changes to session and user retrieval call sites.
  • parse-community/parse-server#10278: Both PRs modify src/Routers/UsersRouter.js handleMe and related REST lookup methods, with parameter removal in this PR overlapping with user/session refetch logic changes in the retrieved PR.

Suggested reviewers

  • Moumouls
🚥 Pre-merge checks | ✅ 5 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is empty; no content was provided by the author, making it impossible to verify compliance with the required template sections like Issue, Approach, and Tasks. Add a pull request description following the provided template, including sections for Issue, Approach, and Tasks to document the security fix and changes made.
Engage In Review Feedback ❓ Inconclusive PR #10463 is freshly created (2026-05-17) and marked open with no assigned reviewers. No review comments appear in git history. GitHub PR review API data inaccessible in sandbox environment. Access GitHub PR #10463 directly to verify if review comments exist and check user engagement through commits or discussion threads.
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Security Check ✅ Passed PR properly fixes GHSA-38m6-82c8-4xfm (ReDoS via client version parsing). ClientSDK module removed, parameters eliminated, regex parsing replaced with deletion. Tests verify large payload protection.
Title check ✅ Passed The pull request title begins with the required 'fix:' prefix and clearly summarizes the main security vulnerability fix addressing polynomial ReDoS via client version parsing.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@spec/vulnerabilities.spec.js`:
- Around line 5950-5970: The two tests that measure elapsedMs around
middlewares.handleParseHeaders (the "X-Parse-Client-Version" and
"_ClientVersion" adversarial cases using buildFakeReq) use strict absolute caps
(<100ms and <500ms) that can cause CI flakiness; relax these assertions to more
tolerant thresholds (for example change the first to
expect(elapsedMs).toBeLessThan(250) and the second to
expect(elapsedMs).toBeLessThan(1000) or use toBeLessThanOrEqual) so the tests
still enforce performance bounds but won’t fail on slower runners.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 991e2e7b-6d4b-44b9-9010-ebe4468e96f8

📥 Commits

Reviewing files that changed from the base of the PR and between 4f90c5e and 13ae85a.

📒 Files selected for processing (18)
  • spec/ClientSDK.spec.js
  • spec/Middlewares.spec.js
  • spec/vulnerabilities.spec.js
  • src/ClientSDK.js
  • src/GraphQL/helpers/objectsMutations.js
  • src/GraphQL/helpers/objectsQueries.js
  • src/GraphQL/loaders/usersQueries.js
  • src/RestQuery.js
  • src/RestWrite.js
  • src/Routers/AggregateRouter.js
  • src/Routers/AudiencesRouter.js
  • src/Routers/ClassesRouter.js
  • src/Routers/IAPValidationRouter.js
  • src/Routers/InstallationsRouter.js
  • src/Routers/SessionsRouter.js
  • src/Routers/UsersRouter.js
  • src/middlewares.js
  • src/rest.js
💤 Files with no reviewable changes (11)
  • src/ClientSDK.js
  • spec/ClientSDK.spec.js
  • src/Routers/InstallationsRouter.js
  • src/Routers/SessionsRouter.js
  • src/Routers/AggregateRouter.js
  • src/Routers/AudiencesRouter.js
  • src/GraphQL/loaders/usersQueries.js
  • spec/Middlewares.spec.js
  • src/Routers/ClassesRouter.js
  • src/Routers/IAPValidationRouter.js
  • src/Routers/UsersRouter.js

Comment thread spec/vulnerabilities.spec.js
@mtrezza
Copy link
Copy Markdown
Member Author

mtrezza commented May 17, 2026

@coderabbitai review your feedback conversations and resolve where you found a consensus

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 17, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

Let me fetch all the review conversations on this PR to identify which ones have reached consensus.


🏁 Script executed:

#!/bin/bash
# Get all review comments made by coderabbitai on PR `#10463`
gh pr view 10463 --repo parse-community/parse-server --json reviews,comments

echo "---REVIEW_THREADS---"
gh api repos/parse-community/parse-server/pulls/10463/comments --paginate | jq '[.[] | {id, body: .body[0:200], user: .user.login, path: .path, line: .line, resolved: false}]'

echo "---ISSUE_COMMENTS---"
gh api repos/parse-community/parse-server/issues/10463/comments --paginate | jq '[.[] | {id, body: .body[0:300], user: .user.login}]'

Length of output: 39327


I found one review thread where consensus was reached — the timing cap discussion on spec/vulnerabilities.spec.js. I explicitly agreed with your rationale, so I'll resolve it now.


🏁 Script executed:

#!/bin/bash
# Get the pull request review thread IDs to resolve
gh api graphql -f query='
{
  repository(owner: "parse-community", name: "parse-server") {
    pullRequest(number: 10463) {
      reviewThreads(first: 20) {
        nodes {
          id
          isResolved
          path
          comments(first: 3) {
            nodes {
              author { login }
              body
            }
          }
        }
      }
    }
  }
}'

Length of output: 12775


@mtrezza All review conversations have been checked — there was only one thread (the timing cap discussion on spec/vulnerabilities.spec.js), and it is already resolved. No further action needed. ✅

@codecov
Copy link
Copy Markdown

codecov Bot commented May 17, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.58%. Comparing base (9fee1a0) to head (646effa).
⚠️ Report is 4 commits behind head on alpha.

Additional details and impacted files
@@            Coverage Diff             @@
##            alpha   #10463      +/-   ##
==========================================
- Coverage   92.58%   92.58%   -0.01%     
==========================================
  Files         194      193       -1     
  Lines       16901    16874      -27     
  Branches      234      234              
==========================================
- Hits        15648    15623      -25     
+ Misses       1230     1228       -2     
  Partials       23       23              

☔ 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.

@mtrezza mtrezza changed the title fix: GHSA-38m6-82c8-4xfm fix: Pre-authentication denial of service via client version header regex backtracking (GHSA-38m6-82c8-4xfm) May 17, 2026
@mtrezza mtrezza merged commit 56c159e into parse-community:alpha May 17, 2026
24 checks passed
parseplatformorg pushed a commit that referenced this pull request May 17, 2026
## [9.9.1-alpha.1](9.9.0...9.9.1-alpha.1) (2026-05-17)

### Bug Fixes

* Pre-authentication denial of service via client version header regex backtracking ([GHSA-38m6-82c8-4xfm](GHSA-38m6-82c8-4xfm)) ([#10463](#10463)) ([56c159e](56c159e))
@parseplatformorg
Copy link
Copy Markdown
Contributor

🎉 This change has been released in version 9.9.1-alpha.1

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label May 17, 2026
@mtrezza mtrezza deleted the fix/GHSA-38m6-82c8-4xfm-9.x branch May 17, 2026 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state:released-alpha Released as alpha version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants