Repository navigation
fix: keep server-managed names out of reach of DAV clients (#38) - #41
Merged
Merged
Conversation
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
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.
What was wrong
A DAV client could delete or overwrite a collection's own metadata file with a
single request. Verified against a local build before the fix:
DELETE /carddav/alice/def-addressbook/.Radicale.props-> 204, the filedisappears from disk, the collection then answers 404, the item inside it
answers 404, the collection vanishes from the home listing and
MKCALENDARon the same href answers 405. There is no way back over DAV.PUT /caldav/alice/def-calendar/.Radicale.propswith a valid iCalendar whoseUIDis.Radicale.props-> 201, metadata replaced by iCalendar bytes,collection gone.
(
.hidden.icswas accepted and listed).What this changes
internal/store: newcleanName, used by every collection and object entrypoint (
GetCollection,CreateCollection,DeleteCollection,UpdateCollectionProps,collDir,ReadItem,WriteItem,DeleteItem,ItemModTime). It rejects the names the server keeps for itself: a leadingdot (
.Radicale.props, the.tmp-*files ofwriteFileAtomic) or a trailingtilde.
ListItemsnow skips those names instead of only.Radicale.props.This is the same rule Radicale applies through
pathutils.is_safe_filesystem_path_componentbefore it touches thefilesystem (
upload.py:40,delete.py:47,get.py:49,153of v3.5.3).internal/dav: reserved names are invisible inresolve(404, like any namethat is not servable) and refused in
resolveForWriteandresolveForCreatewith 400, which is what Radicale answers to a PUT with such an href.
Tests
internal/store:TestReservedNamesAreRefusedcovers read, write, delete,collection lookup and creation, that the metadata file survives a delete
attempt, and that the listing hides hidden names.
cmd/server:TestServerManagedNamesAreUnreachabledrives the real handlerover HTTP: PUT 400, DELETE 404, GET 404, hidden item 400, reserved collection
400, metadata untouched and the collection still answering 207.
Also checked end to end against a copy of the production volume (temporary
directory, production untouched): the real item keeps being served, the attack
answers 404/404/400, an ordinary
PUT/DELETEstill answers 201/204, and theinherited
.Radicale.cachedirectories do not leak into listings.Closes #38