cenkalti / rain / 37147560904

51%

Build Type

push

github

Committed by cenkalti

Commit Message

metainfo: defuse torrent names used as paths

A torrent name reaches the filesystem in three places, unsanitized.
CleanName replaced separators but left a bare ".." alone, and filepath
reads that as navigation rather than as a file name.

Write side, reachable with the default config: a torrent named ".." got
Files[0].Path of "../a.txt", so its data landed one level above its own
directory -- in the shared data dir by default, and outside the data dir
when data-dir-includes-torrent-id is off.

Delete side: Session.stopAndRemoveData joined the raw name onto DataDir
and called os.RemoveAll. With the name ".." that is RemoveAll on the
parent of the data dir, which takes the data dir and everything beside
it. Reaching it needs data-dir-includes-torrent-id off, which is not the
default, plus a remove with keepData false.

Three changes:

  - CleanName now renames "." and ".." to "_" and "__". After separator
    replacement those are the only values left that can move a path. A
    name that merely contains dots, or is padded like " .. ", is an
    ordinary file name and is untouched.
  - stopAndRemoveData cleans the name, which is also what the data was
    written under, so removal stops missing the real directory whenever
    the raw and cleaned names differ.
  - removeDataDir refuses any destination that is not inside DataDir, so
    a recursive delete driven by a torrent-supplied name has a guard
    that does not depend on the cleaning being exhaustive.

Info.Name stays verbatim for display and the RPC surface; only paths
built from it are cleaned. The resume database filename in the download
command is cleaned too.

Each guard was verified to be load-bearing by disabling it and watching
the tests fail.

Coverage Stats

26 of 34 new or added lines in 3 files covered. (76.47%)

8 existing lines in 4 files now uncovered.

6127 of 12040 relevant lines covered (50.89%)

121.33 hits per line