Add Cache Poisoning Vulnerability#534
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #534 +/- ##
============================================
+ Coverage 52.01% 53.12% +1.10%
- Complexity 465 510 +45
============================================
Files 69 72 +3
Lines 2676 2850 +174
Branches 285 296 +11
============================================
+ Hits 1392 1514 +122
- Misses 1153 1201 +48
- Partials 131 135 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…ature/cache-poisoning # Conflicts: # src/main/java/org/sasanlabs/vulnerability/types/VulnerabilityType.java # src/main/resources/i18n/messages.properties
preetkaran20
left a comment
There was a problem hiding this comment.
@luks-santos I must say this is amazing code change. Very thorough PR. I really liked the PR completeness. I have added small comments, can you please address?
|
@preetkaran20 Thanks a lot for the feedback! |
- Level 4 cached response now embeds derived email and last-login IP - Rewrite L3, L4 and L5 payload hints around the lab UI flow - Auto-clear attacker input after poison click (per-level)
…ature/cache-poisoning # Conflicts: # src/main/resources/i18n/messages.properties
|
I've pushed the changes addressing the feedback. Summary of what was implemented: Frontend / UX
Vulnerability depth
The PR description has been updated to reflect the final scope. Let me know if there's anything else you'd like me to adjust! |
preetkaran20
left a comment
There was a problem hiding this comment.
Overall, @luks-santos this is really a great PR. I have added a few comments, I think once they are resolved, i think we can merge it quickly. Just a few thoughts on browser side caching issues which we need to work on.
…t keyed by level, so that clearing only evicts entries for the requested level instead of flushing the whole shared cache.
Introduces a per-level toggle that switches between two Cache-Control modes on the server response: - ON (default, realistic): public, max-age=60 — browser and shared cache - OFF (student-friendly): public, s-maxage=60, max-age=0 — shared cache only, browser always revalidates, scanners still detect via s-maxage
|
Hi @preetkaran20, I've pushed the changes addressing the feedback. Here's a principal summary of what was implemented: Cache reset reliability
Browser cache toggle (Levels 1–4)
This ensures the browser always revalidates while the shared cache layer (the one being poisoned) keeps its 60-second TTL.
Let me know if there's anything else you'd like me to adjust! I've opened the PR at SasanLabs/VulnerableApp-facade#112 |
preetkaran20
left a comment
There was a problem hiding this comment.
Amazing job @luks-santos One of the best change.
|
Merged the PR @luks-santos Amazing work!!!!! |
|
🎉 Thanks for contributing @luks-santos! Also consider ⭐ starring the repo if you like it! 🎯 Want to keep going? Here's a good next issue for you: |
|
@all-contributors please add @luks-santos for code |
|
I've put up a pull request to add @luks-santos! 🎉 |
Summary
Closes #516.
This PR delivers the full level progression for the Cache Poisoning vulnerability, from the basic route-only cache key issue to the secure implementation, and adds the supporting frontend, hints, i18n, and automated tests
needed for the lab experience.
Implemented levels
bannerquery parameter is rendered unescaped (also exploitable as stored XSS via the shared cache) while the cache key uses only the route.bannerparameter does not fix the underlying issue because the cache key is still route-only — alternative unkeyed inputs keep the poisoning viable.X-Forwarded-Hostheader used to build absolute asset URLs.Cache-Control: private, no-store.