fix(core): Fix malicious escaping of dimensions.#3599
Conversation
📝 WalkthroughWalkthroughThis PR adds URL-encoding and URL-decoding to Casbin v2 dimension serialization, parsing, and matching to prevent dimension injection attacks. Core functions now escape values during serialization and unescape them during comparison. Unit tests validate encoding safety and round-trip correctness. A BDD scenario confirms the authorization system rejects keys with injected dimension characters. ChangesCasbin v2 dimension URL encoding for injection prevention
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Dependency ReviewThe following issues were found:
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a security vulnerability in the authorization system where malicious KAS URI values could be manipulated to inject additional authorization dimensions. By applying URL-encoding to dimension values during serialization and decoding them during parsing, the system now ensures that complex URI strings are treated as single, atomic values, preventing unauthorized policy matching. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. When query strings hide a dark snare, / We encode them with diligent care. / No injection can pass, / In our Casbin, alas, / The logic is now tight and fair. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces URL-encoding and decoding for dimension values during serialization and parsing to prevent injection vulnerabilities from special characters like '&' and '='. It also adds comprehensive unit and BDD tests to verify this behavior. The review feedback highlights a matching discrepancy where policy dimensions in dimensionMatch are not unescaped before comparison, making it impossible to match policies containing escaped characters, and suggests unescaping the policy values prior to comparison.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Signed-off-by: Chris Reed <creed@virtru.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
| } | ||
| policyWildcard := policyVal == "*" | ||
| if !policyWildcard { | ||
| unescapedVal, err := url.QueryUnescape(policyVal) |
There was a problem hiding this comment.
claude comment -- this turns "+" into a space, not sure if thats a case we need to account for, if so we could use url.PathUnescape instead
There was a problem hiding this comment.
I actually decided to change the method for how we are doing the escaping instead. Before, I was expecting customers to have to url escape their FQNs but that would be unreadable. I was serializing all values to prevent malicious actors from taking advantage of the policy dimension parsing (&). Instead now I just require that the following special characters are escaped if used in the dimension value literally: [&, *, %].
elizabethhealy
left a comment
elizabethhealy
left a comment
There was a problem hiding this comment.
lgtm, if the above comment isnt a case we need to handle
Signed-off-by: Chris Reed <creed@virtru.com>
policy-bot-opentdf
Bot
dismissed
elizabethhealy’s stale review
Invalidated by push of 02e2ee4
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
|
2 participants
Summary
Fixes KAS URI dimension injection in authz v2 by escaping dimension values before Casbin matching, then decoding them during dimension parsing. This prevents harmful values containing dimension parser tokens from being interpreted as additional authorization dimensions.
Changes
%as%25&as%26*as%2A=,+, URI schemes, paths, and query keys remain unchanged.&and=+, spaces,%, and*dimensionMatchbehavior with escaped policy dimensions and wildcard valueskas-ascoped client cannot access KAS keys whose URI injects a trailingkas_uri=https://kas-a.example.com.Security Impact
Previously, a malicious KAS URI like:
could be serialized into dimensions in a way that allowed the trailing
kas_uripair to overwrite the original value during parsing. The fix keeps the full URL as one dimension value, so policy matching compares against the actual KAS URI.Policy Authoring Note
Raw policy dimensions remain readable. Authors only need percent-encoding for values containing characters that conflict with the dimension grammar, especially literal
&inside a value:Raw
*remains the wildcard policy value; use%2Afor a literal*.💡 Follow-up Recommendation: Structured Policy Dimensions