search

adafruit / Adafruit_CircuitPython_HTTPServer

Simple HTTP Server for CircuitPython
MIT License
46 stars 30 forks source link

Prevent parent directory access, custom Errors #49

Closed michalpokusa closed 1 year ago

michalpokusa commented 1 year ago

~~Part of v3 Milestone, contains some minor breaking changes If possible, please do not make it a separate "Release"~~

This PR prevents accessing files outside root server directory, Previously, when there was a ".." in path it was possible to do that, which was unsecure as users could access files with configuration or secrets, or any other file.

All paths are now checked by default, whether they contain ".." or "\" (backslash) inside. if that is teh sace a 403 Forbidden is returned.

In order to keep everything readable in clean, I intruduced custom exceptions, instead of raising e.g. RunTimeError or ValueError with a comment, now InvalidPathError or ResponseAlreadySentError.

Server's root_path is now specified in constructor, not in server_forever or pool methods.

Some minor refactor to separate logic for diffrent smaller tasks inside HTTPResponse.

Updated examples to use new constructor and to use settings.toml instead of secrets.py.

michalpokusa commented 1 year ago

Baseline testing with adafruit_httpserver 2.5.0, which allows serving static files from the filesystem root_path without a route. An explicit route will allow accessing a file outside of root_path.

But also, it is possible using a TCP socket to access files outside of the root_path without a route (by adding ../ to a path, for example). Thanks @Neradoc for identifying that in Discord. Typical browsers, and curl, filter that.

The current PR disallows ".." (and backslash) in a path. Tested successfully with: Adafruit CircuitPython 8.1.0-beta.1-31-g4e25a4f6b on 2023-04-18; Adafruit QT Py ESP32S2 with ESP32S2. It is still possible for the user to explicitly allow "/" as the root_path.

LGTM

Thank you for testing @anecdata.

Just a note. It is still also possible to do an explicit route to serve file outside root_path.

michalpokusa commented 1 year ago

Thank you for merging. I see that teh changes were released under 3.0.0 and the 3.0.1. Considering I have some more changes, improvements, some of which are not backwards compatible it might be necessary to release that the as 4.0.0. Maybe it is better to do it this way as it might take some time to review and correct everything in incoming 4.0.0 PR.