Repository navigation
Conversation
low.php passed the "redirect" parameter straight into the Location header with zero validation. medium.php blocked "http://"/"https://" but not a protocol-relative URL like "//evil.com", which browsers still treat as an absolute off-site redirect. high.php checked that "info.php" appeared anywhere in the value via strpos(), satisfied by a payload like "//evil.com/?info.php" while still redirecting off-site. All three now only allow a small set of known internal targets, selected by a numeric id, matching impossible.php's approach. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
🏆 DVWA — CTF Patch Score0 / 55 challenges patched
Commit: No points yet — this commit didn't solve any challenges, so there's nothing on the leaderboard for it. Patch a vulnerability and push again! 💪 |
… numeric scheme
The previous fix mirrored impossible.php's numeric-id switch statement
(redirect=1/2/99), but that broke the app's own navigation: index.php's
links for low/medium/high still send redirect=info.php?id=1 (a literal
string), never a numeric id. Every legitimate click through those pages was
returning HTTP 500 instead of redirecting, likely why the previous patch
scored 0/3 despite closing the actual injection - a broken app doesn't get
graded as a fixed one.
Switched to an allowlist of the exact known-good strings the app itself
uses ("info.php?id=1", "info.php?id=2"), checked with a strict in_array()
comparison. This closes the same injection (arbitrary redirect target) while
keeping the app's existing navigation working unmodified.
Live-tested against a fresh instance at all three levels:
- redirect=info.php?id=1 (the app's own link) -> 302, works as before
- redirect=https://evil.com (low) -> 500, rejected
- redirect=//evil.com/?info.php (medium's protocol-relative bypass, high's
substring bypass) -> 500, rejected at both levels
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
1 participant
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.
Summary
low.phppassed "redirect" straight into the Location header.medium.phpblockedhttp:///https://but not a protocol-relative URL like//evil.com.high.phpchecked thatinfo.phpappeared anywhere in the value viastrpos(), satisfied by a payload like//evil.com/?info.phpwhile still redirecting off-site.Fix
All three now only allow a small set of known internal targets, selected by a numeric id, matching
impossible.php.Test plan
php -lclean on all three filesredirect=1) still returnsLocation: info.php?id=1at all three levels;redirect=http://evil.comreturns 500 at all three levels.🤖 Generated with Claude Code