Verify fix for Guest Token IDOR vulnerability - #1
Draft
google-labs-jules[bot] wants to merge 6 commits into
Draft
google-labs-jules[bot] wants to merge 6 commits into
google-labs-jules[bot] wants to merge 6 commits into
Conversation
This submission confirms that the Insecure Direct Object Reference (IDOR) vulnerability in guest token chart access has been resolved. - Verified that `tests/integration_tests/security/guest_token_security_tests.py` exists and contains `test_raise_for_access__chart_not_on_dashboard`. - Ran the test successfully, confirming that attempting to access a chart not associated with the authorized dashboard raises a `SupersetSecurityException`. - Identified PR apache#25081 as the one that enabled the secure logic for guest/embedded users. - Confirmed that PR apache#24789 introduced the underlying security logic (Dashboard aware RBAC). The vulnerability report is valid for older versions, but the issue is fixed in recent versions.
Author
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
- Confirmed that the IDOR vulnerability allowing guest tokens to access charts from unauthorized dashboards is fixed. - The fix was introduced in PR apache#25081 and released in Apache Superset 3.0.0. - Verified by running existing integration tests in `tests/integration_tests/security/guest_token_security_tests.py` which explicitly check for this unauthorized access scenario and raise a `SupersetSecurityException`. - No code changes were needed as the fix is already present in the codebase.
Confirmed that the vulnerability is fixed and identified the specific versions and Pull Requests that addressed different aspects of the issue: - **Strict IDOR (Accessing unauthorized charts):** - **Fixed in Version:** 3.0.0 - **PR:** apache#25081 ("fix: Allow embedded guest user datasource access with dashboard context") - **Mechanism:** `raise_for_access` now validates that the chart belongs to the dashboard. - **Payload Tampering (Modifying metrics/columns):** - **Fixed in Version:** 4.0.0 - **PR:** apache#27484 ("fix: check if guest user modified query") - **Mechanism:** Introduced `query_context_modified` check. - **Information Disclosure (Schema Leaks):** - **Fixed in Version:** 5.0.0 - **PR:** apache#30858 ("fix(chart data): removing query from /chart/data payload when accessing as guest user") Verified by running existing integration tests in `tests/integration_tests/security/guest_token_security_tests.py` and temporarily adding a test case to simulate payload tampering, which successfully triggered a security exception.
- Confirmed strict IDOR fix (accessing unauthorized charts) was in 3.0.0 (PR apache#25081). - Confirmed payload tampering fix (modifying metrics) was in 4.0.0 (PR apache#27484). - Confirmed schema leak fix was in 5.0.0 (PR apache#30858). - Verified via integration tests.
This commit confirms the fix for the reported Guest Token IDOR vulnerability and payload tampering issues. Findings: - **Strict IDOR (Accessing unauthorized charts):** - **Fixed in Version:** 3.0.0 - **PR:** apache#25081 - **Verification:** Verified with standalone PoC `poc_idor.py`. Attempting to access a chart belonging to Dashboard B while authorized for Dashboard A raises a security exception. - **Payload Tampering (Modifying metrics/columns):** - **Fixed in Version:** 4.0.0 - **PR:** apache#27484 - **Verification:** Verified with standalone PoC `poc_idor.py`. Attempting to modify metrics for an authorized chart raises a security exception. - **Schema Leak:** - **Fixed in Version:** 5.0.0 - **PR:** apache#30858 Verification method: - Created and executed a standalone Proof-of-Concept script (`poc_idor.py`) that bootstraps the Superset app context, creates temporary dashboards/charts, issues a guest token, and attempts the exploits. Both exploits were blocked. - Ran existing integration tests in `tests/integration_tests/security/guest_token_security_tests.py`. No code changes required as fixes are already present.
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The reported IDOR vulnerability involving guest tokens accessing charts outside of their authorized dashboard has been verified as fixed.
I confirmed the fix by:
test_raise_for_access__chart_not_on_dashboardintests/integration_tests/security/guest_token_security_tests.py.SupersetSecurityExceptionis raised.No code changes were required as the fix is already present in the codebase. This submission documents the verification process.
PR created automatically by Jules for task 7429293929104773877 started by @sha174n