-
Notifications
You must be signed in to change notification settings - Fork 38
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
Do not allow access to resources outside of static. #923
Conversation
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.
I'm usually a little more radical in my webserver implementations. All browsers automatically collapse relative paths before the request is made.
So in addition to any logic you wrote, if any of the request path components is a "." or ".." you can be sure that this is a malicious hand-crafted request.
On Windows, this fails because the string passed to Adding this line seems to fix it in windows: |
I have to think about this. |
Agreed. :) |
I made a commit that should make this work on Windows too, but I haven't tested it on either OS. |
Tested the change on Linux and works. Because browsers collapse $ netcat localhost 8080
GET /static/../main.py?token=[TOKEN] HTTP/1.1
Host: localhost
(you need two newline at the end) With this PR it will give a 403 response HTTP/1.0 403 Forbidden
Server: BaseHTTP/0.6 Python/3.6.1
Date: Tue, 12 Dec 2017 20:54:41 GMT
Content-type: text/html
Set-Cookie: _session_id=1e2134ff11a84d8775629e6f2e108c91cfc349ca
whereas master will provide HTTP/1.0 200 OK
Server: BaseHTTP/0.6 Python/3.6.1
Date: Tue, 12 Dec 2017 20:54:01 GMT
Content-Type: text/x-python
Content-Length: 4129
Set-Cookie: _session_id=489cd5aa032ae1a5b1c625659685cc0d24970a63
import argparse
import errno
import logging
import os.path
import threading
import webbrowser
... |
Nice! That's incredibly useful -- thank you... It works on Windows, btw... :) |
788d451
to
a876b11
Compare
This should have been merged ages ago.... thank you, @jgosmann , it's merged now! |
This is another security issue. By providing paths like
/static/../__init__.py
it was possible to access arbitrary files on the file system. Inmaster
this requires at least authentication, but in #921 I'm removing the constraint because the favicon should be available without login and we might want to use some of the style sheets on the login page at some point.