Magic Folder: enforce that downloaded files are only written below the magic folder directory

[daira]

​https://github.com/tahoe-lafs/tahoe-lafs/blob/665c36e45cab3487f279ccae44f79f606613c8f2/src/allmydata/frontends/magic_folder.py#L539

Since name comes from another client's DMD and is an arbitrary string, it can be an absolute path, or contain ".." path elements, or start with "~", which would cause the file to be written somewhere that might not be below this client's magic folder directory.

We should use FilePath.preauthChild to prevent this, and to provide a clearer type distinction between relative and absolute paths.

[daira]

To be clear, this is not a security vulnerability in master or any released version, only in some magic folder development branches.

[dawuud]

I've extended the test_alice_bob test to use an insecure path... however it is unclear where this InsecurePath? exception will surface.. and therefore I am not sure what the proper way to test for it...

I *could* make the test check for the existence of the offending file outside of the magic-folder directory... this might be sufficient to test for the bad behavior... but it would stop working when we fix our code to fire the failure containing the exception...

this dev branch here: ​https://github.com/david415/tahoe-lafs/tree/2506.only-write-to-download-dir.0

[dawuud]

Recently Daira made this dev branch so that twisted filepath objects are used more often than file path unicode strings:

​https://github.com/tahoe-lafs/tahoe-lafs/tree/2506.enforce-paths.0

I was working with the code trying to fix it and I discovered that the twisted inotifier doesn't produce filepath objects in unicode mode... and this breaks our code. Here's the twisted bug regarding this issue:

​https://twistedmatrix.com/trac/ticket/7928

[dawuud]

just in helps here's my sloppy dev branch with some changes that helped me identify problems ​https://github.com/david415/tahoe-lafs/tree/2506.enforce-paths.0

[dawuud]

my dev branch here shows a weird bug... and the test now fails instead of live-locking: ​https://github.com/david415/tahoe-lafs/tree/2506.enforce-paths.1

i made the test fail by testing for the local magic-folder db version of the file after alice rewrites it...

[daira]

Replying to dawuud:

Recently Daira made this dev branch so that twisted filepath objects are used more often than file path unicode strings:

​https://github.com/tahoe-lafs/tahoe-lafs/tree/2506.enforce-paths.0

That branch doesn't yet use FilePaths in all the cases I wanted to. I have a branch on my machine that does, and hopefully we can work on that during our pairing today once we've fixed the current bug (or if we think it will help with the bug).

[dawuud]

Last night Daira and I paired and she fixed our design doc: ​https://github.com/david415/tahoe-lafs/commit/c649721318485cac7496cb37a3ff003be4d5234c

This morning I fixed the code... all tests now pass: ​https://github.com/david415/tahoe-lafs/tree/2506.enforce-paths.2

I think this ticket can be closed now... pending review.

[daira]

Reviewed. Looks good so far, but this needs a test that InsecurePath is actually raised (instead of writing the file) when it should be.

[daira]

Arrgh, FilePath.preauthChild is insecure: ​https://twistedmatrix.com/trac/ticket/6527

[daira]

I have a fix for this, but it's entangled with other changes that cause test failures, so I haven't pushed it yet. I will try to push it tonight.

[daira]

I believe this is now fixed on the ​https://github.com/tahoe-lafs/tahoe-lafs/commits/2506.enforce-paths.2 branch. It passes existing tests, but still lacks a specific test for the security bug.

I have also rebased to master: ​https://github.com/tahoe-lafs/tahoe-lafs/commits/2506.enforce-paths.4.

[daira]

Note that these include the clock improvements on your branch, David.

[daira]

This is now tested, in the Alice_tries_to_p0wn_Bob section of test_alice_bob.

[Brian Warner <warner@…>]

In cec315d/trunk:

Add test that we don't write files outside the magic folder directory. refs ticket:2506

Signed-off-by: Daira Hopwood <daira@…>