-
Notifications
You must be signed in to change notification settings - Fork 286
static: clean the path URL before redirecting #199
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report
@@ Coverage Diff @@
## master #199 +/- ##
=======================================
Coverage 91.77% 91.77%
=======================================
Files 11 11
Lines 1556 1557 +1
=======================================
+ Hits 1428 1429 +1
Misses 96 96
Partials 32 32
|
Thanks for the PR! But I am not confident this is the right change to fix #198. I think we should instead do this on line 126 (not verified): -file := ctx.Req.URL.Path
+file := path.Clean(ctx.Req.URL.Path) Besides, can you add some tests to catch this case? |
I can't see your reply here. Why |
I don't know why it doesn't solve it. Let me push with the test and check it out. |
This prevents a malicious redirect with a crafted URL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice,
static.go
Outdated
@@ -123,7 +123,7 @@ func staticHandler(ctx *Context, log *log.Logger, opt StaticOptions) bool { | |||
return false | |||
} | |||
|
|||
file := ctx.Req.URL.Path | |||
file := path.Clean(ctx.Req.URL.Path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the call of path.Clean
here is ineffective and needless, so we can remove it.
Sorry about noises 😅 Waiting for CI to merge. |
Hi, this seems to have started causing issues when |
Hello Matt,
Could you please create this as an issue, including an example so I am
able to reproduce your issue?
Thank you.
|
This prevents a malicious redirect with a crafted URL. Resolves #198.
It prevents links such as:
http://localhost:4000/%2fhumaidq.ae%2f..
http://localhost:4000//humaidq.ae%2f..
from giving a 302 Found redirecting to
humaidq.ae