jk icon indicating copy to clipboard operation
jk copied to clipboard

Ensure symlinks cannot be used to circumvent read baseDirectory

Open dlespiau opened this issue 5 years ago • 6 comments

We need to resolve symlinks when checking the read() path doesn't go beyong baseDirectory, otherwise the hermetic constraint can be worked around with symlinks point to other parts of the filesystem.

dlespiau avatar Jan 30 '19 14:01 dlespiau

What's the er, attack you have in mind?

squaremo avatar Jan 30 '19 15:01 squaremo

We have an index.js script with:

import * as foo './foo/index.js';

The fs looks like:

/index.js
/foo/   ->   ../../outside/

and run jk run ./index.js

dlespiau avatar Jan 30 '19 15:01 dlespiau

Well that's module resolution, rather than reads, but same sort of thing. I see what is happening, what I want to understand is how someone could (maliciously) use it to provoke an unexpected result. If I'm pointing jk at a bit of filesystem, presumably I'm OK with the symlinks therein?

squaremo avatar Jan 30 '19 19:01 squaremo

That's definitely a possible argument to make :) We could decide either:

a. If there's a symlink there, it's for a good reason and we don't want to get in the way of developers

b. We want keep plugging ways of making the scripts non hermetic. Otherwise we just lose that property on any big enough code base, developers will get creative. We could still provide command line options to opt out, in this case, an option to be able to read from anywhere on the filesystem. This way we can try and preserve the hermetic property while allowing to opt out if people want to do their thing,

dlespiau avatar Jan 31 '19 18:01 dlespiau

I would like to suggest an extension of this problem:

What if ./foo/index.js were a named pipe? (as per Unix file types) Also, ./foo could be a bound (mount --bind) directory.

Both would allow data to be leaked from the outside. :) A solution that comes up to my mind is to whitelist the directory structure under the base dir (allow only regular files and "well behaved" directories), and only allow reads from the whitelisted files.

mflendrich avatar Feb 14 '19 21:02 mflendrich

At some point we have to draw a line :) "hermeticity" isn't supposed to be a security feature or completely locked down. Currently we define it as: "if a git repository with the configuration is cloned and jk is run, the output should be the same on any machine".

In that context, I singled out symlinks because it is an object that can be stored into a git repo.:

$ ll
drwxrwxr-x  8 damien damien 4096 Feb 15 12:17 .git/
lrwxrwxrwx  1 damien damien   16 Feb 15 12:16 resolv.conf -> /etc/resolv.conf

$ git show
commit a4f5cbe2af54ae9a923e3f323854f166277a02c2
Author: Damien Lespiau <[email protected]>
Date:   Fri Feb 15 12:16:24 2019 +0000

    Add symlink to resolve.conf

diff --git a/resolv.conf b/resolv.conf
new file mode 120000
index 0000000..9d5673c
--- /dev/null
+++ b/resolv.conf
@@ -0,0 +1 @@
+/etc/resolv.conf
\ No newline at end of file

That said, for security purposes, it may still be a good idea to run the process producing configuration in some kind of isolation, say a mount namespace with mount propagation turned off.

dlespiau avatar Feb 15 '19 12:02 dlespiau