Repository navigation
[security] GHSA-q896-wrpm-3fwc: SQL injection via the GraphQL listing filter argument (2.4 backport) - #1186
Closed
mcop1 wants to merge 1 commit into
Closed
[security] GHSA-q896-wrpm-3fwc: SQL injection via the GraphQL listing filter argument (2.4 backport)#1186mcop1 wants to merge 1 commit into
mcop1 wants to merge 1 commit into
Conversation
… filter argument (2.4 backport) Helper::buildSqlCondition() returned any string it was given unchanged, so a JSON string anywhere in the "filter" argument of getAssetListing or the data object listings ended up in the listing's WHERE clause as raw SQL. Only JSON objects and arrays of objects are accepted now; anything else is rejected with a ClientSafeException. Backport of #1185 from 1.9: the new test file carries this line's POCL header and the upgrade note sits in this line's patch section. (cherry picked from commit c60c0b4) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues block the GraphQL filter-validation backport.
No open issues 🎉
What changed in this PR
Backports the GraphQL listing filter SQL injection fix to the 2.4 line. The change rejects raw SQL strings in filter conditions while preserving documented JSON filter syntax.
Changes:
- Validates condition types recursively and skips non-object entries during join processing.
- Adds regression tests for valid filters and rejection of non-object conditions.
- Documents the filter restrictions in the
2.4.7upgrade notes.
| File | Description |
|---|---|
tests/GraphQL/Resolver/AssetListingTest.php |
Tests valid asset filtering and rejection of raw-string filters. |
tests/GraphQL/HelperTest.php |
Adds SQL generation, invalid-condition, and join-processing tests. |
src/GraphQL/Helper.php |
Rejects invalid condition types and guards join processing. |
doc/01_Installation_and_Upgrade/01_Upgrade_Notes.md |
Explains the filter restrictions and client-facing error. |
Review effort: Balanced
Contributor
Author
|
Superseded by forward merge 1.9 → 2.4 → 2026.3 → 2026.x of #1185. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Backport of #1185 to the
2.4line (platform version 2025.4, LTS until December 2028).Vulnerability
Pimcore\Bundle\DataHubBundle\GraphQL\Helper::buildSqlCondition()returned any string it received unchanged. Thefilterargument ofgetAssetListingand of the data object listings is JSON-decoded and passed to this builder, so a JSON string at the top level or nested inside$and/$or/ a field array ended up in the listing'sWHEREclause as raw SQL.Fix
Same change as #1185:
buildSqlCondition()only accepts JSON objects and arrays of objects, at every level, and rejects anything else with aClientSafeException("invalid filter: conditions must be JSON objects");addJoins()skips non-object entries.Differences to the
1.9commit:tests/GraphQL/HelperTest.phpcarries this line's POCL license header.2.4.7(only conflict of the cherry-pick).Backward compatibility
Helperis@internal. Every filter in the documented syntax produces the same SQL as before; only undocumented raw-string filters are rejected now.Release notes
Testing
Verified locally against this line's dependencies (
pimcore/pimcorecontainer + MariaDB 10.11): full Codeception suite green, including the newHelperTest(11/11) andAssetListingTestcases; PHPStan (project config) and php-cs-fixer clean.🤖 Generated with Claude Code