diff --git a/sshfs.c b/sshfs.c index 1bb83c76..ef8c5017 100644 --- a/sshfs.c +++ b/sshfs.c @@ -313,6 +313,7 @@ struct sshfs { int fstat_workaround; int createmode_workaround; int transform_symlinks; + int contain_symlinks; int follow_symlinks; int no_check_root; int detect_uid; @@ -493,6 +494,8 @@ static struct fuse_opt sshfs_opts[] = { SSHFS_OPT("sshfs_debug", debug, 1), SSHFS_OPT("reconnect", reconnect, 1), SSHFS_OPT("transform_symlinks", transform_symlinks, 1), + SSHFS_OPT("contain_symlinks", contain_symlinks, 1), + SSHFS_OPT("no_contain_symlinks", contain_symlinks, 0), SSHFS_OPT("follow_symlinks", follow_symlinks, 1), SSHFS_OPT("no_check_root", no_check_root, 1), SSHFS_OPT("password_stdin", password_stdin, 1), @@ -2175,6 +2178,36 @@ static void strip_common(const char **sp, const char **tp) } while ((*s == *t && *s) || (!*s && *t == '/') || (*s == '/' && !*t)); } +/* + * Reject symlink targets that could escape the mount root: absolute + * paths and any target containing a ".." component. Returns 1 if + * the target is safe to expose to the kernel, 0 otherwise. + */ +static int symlink_target_is_contained(const char *target) +{ + const char *p = target; + + if (*p == '/') + return 0; + + while (*p) { + const char *comp = p; + + while (*p && *p != '/') + p++; + /* + * Reject any ".." rather than try to normalize: in an + * adversarial filesystem the server controls intermediate + * components, so lexical normalization cannot be trusted. + */ + if (p - comp == 2 && comp[0] == '.' && comp[1] == '.') + return 0; + while (*p == '/') + p++; + } + return 1; +} + static void transform_symlink(const char *path, char **linkp) { const char *l = *linkp; @@ -2239,6 +2272,13 @@ static int sshfs_readlink(const char *path, char *linkbuf, size_t size) buf_get_string(&name, &link) != -1) { if (sshfs.transform_symlinks) transform_symlink(path, &link); + if (sshfs.contain_symlinks && + !symlink_target_is_contained(link)) { + free(link); + buf_free(&name); + buf_free(&buf); + return -EPERM; + } strncpy(linkbuf, link, size - 1); linkbuf[size - 1] = '\0'; free(link); @@ -3720,6 +3760,9 @@ static void usage(const char *progname) " -o passive communicate over stdin and stdout bypassing network\n" " -o disable_hardlink link(2) will return with errno set to ENOSYS\n" " -o transform_symlinks transform absolute symlinks to relative\n" +" -o contain_symlinks reject absolute symlinks and symlinks containing ..\n" +" (enabled by default; disable with no_contain_symlinks)\n" +" -o no_contain_symlinks allow all symlink targets including absolute and ..\n" " -o follow_symlinks follow symlinks on the server\n" " -o no_check_root don't check for existence of 'dir' on server\n" " -o password_stdin read password from stdin (only for pam_mount!)\n" @@ -4277,6 +4320,7 @@ int main(int argc, char *argv[]) sshfs.max_conns = 1; sshfs.ptyfd = -1; sshfs.dir_cache = 1; + sshfs.contain_symlinks = 1; sshfs.show_help = 0; sshfs.show_version = 0; sshfs.singlethread = 0; @@ -4327,6 +4371,12 @@ int main(int argc, char *argv[]) exit(1); } + if (sshfs.transform_symlinks && sshfs.contain_symlinks) + fprintf(stderr, "warning: transform_symlinks with " + "contain_symlinks may reject transformed links " + "containing '..' - consider adding " + "-o no_contain_symlinks\n"); + if (sshfs.idmap == IDMAP_USER) sshfs.detect_uid = 1; else if (sshfs.idmap == IDMAP_FILE) { diff --git a/sshfs.rst b/sshfs.rst index 69e4f189..599e7e5d 100644 --- a/sshfs.rst +++ b/sshfs.rst @@ -175,6 +175,21 @@ Options ``/foo/bar/com`` is a symlink to ``/foo/blub``, SSHFS will transform the link target to ``../blub`` on the client side. +-o contain_symlinks + reject symlink targets that are absolute or contain ``..`` + components. When a blocked symlink is encountered, readlink + returns EPERM. This is enabled by default to prevent a + malicious server from inducing local file reads or writes + through crafted symlink targets. Note that this is stricter + than ``transform_symlinks``: the two options should not normally + be combined, since transformed results often contain ``..`` + and would be rejected by containment. + +-o no_contain_symlinks + disable symlink containment and allow all symlink targets + through unchanged, including absolute paths and paths + containing ``..``. Only use this with fully trusted servers. + -o follow_symlinks follow symlinks on the server, i.e. present them as regular files on the client. If a symlink is dangling (i.e, the target does diff --git a/test/test_sshfs.py b/test/test_sshfs.py index 7c5e4156..c4cc64fb 100755 --- a/test/test_sshfs.py +++ b/test/test_sshfs.py @@ -15,6 +15,7 @@ import filecmp import errno from tempfile import NamedTemporaryFile +from contextlib import contextmanager from util import ( wait_for_mount, umount, @@ -123,6 +124,9 @@ def test_sshfs( # FUSE Cache cmdline += ["-o", "entry_timeout=0", "-o", "attr_timeout=0"] + # Disable containment so tst_symlink can test absolute targets + cmdline += ["-o", "no_contain_symlinks"] + if multiconn: cmdline += ["-o", "max_conns=3"] @@ -305,6 +309,12 @@ def tst_symlink(mnt_dir): assert fstat.st_nlink == 1 assert linkname in os.listdir(mnt_dir) + # Relative symlink without .. should also work + linkname2 = name_generator() + fullname2 = mnt_dir + "/" + linkname2 + os.symlink("subdir/file", fullname2) + assert os.readlink(fullname2) == "subdir/file" + os.unlink(fullname) assert linkname not in os.listdir(mnt_dir) @@ -910,3 +920,163 @@ def test_bad_sftp_reply_len(tmpdir): ) assert res.returncode != 0 assert "bad reply len: 0" in res.stderr + + +@contextmanager +def _sshfs_mount(src_dir, mnt_dir, extra_opts=None): + """Mount src_dir via sshfs, yield, then unmount.""" + cmdline = base_cmdline + [ + pjoin(basename, "sshfs"), "-f", + f"localhost:{src_dir}", mnt_dir, + "-o", "entry_timeout=0", "-o", "attr_timeout=0", + ] + if extra_opts: + for opt in extra_opts: + cmdline += ["-o", opt] + new_env = dict(os.environ) + new_env["G_DEBUG"] = "fatal-warnings" + mount_process = subprocess.Popen(cmdline, env=new_env) + try: + wait_for_mount(mount_process, mnt_dir) + yield mnt_dir + except Exception: + cleanup(mount_process, mnt_dir) + raise + else: + umount(mount_process, mnt_dir) + + +def test_contain_symlinks(tmpdir, capfd) -> None: + """Default containment: safe symlinks resolve, dangerous ones get EPERM.""" + + capfd.register_output(r"^Warning: Permanently added 'localhost' .+", count=0) + _check_ssh_localhost() + + mnt_dir = str(tmpdir.mkdir("mnt")) + src_dir = str(tmpdir.mkdir("src")) + + os.makedirs(pjoin(src_dir, "sub")) + with open(pjoin(src_dir, "sub", "target"), "w") as f: + f.write("hello") + + os.symlink("sub/target", pjoin(src_dir, "safe")) + os.symlink("./sub/target", pjoin(src_dir, "safe_dot")) + os.symlink("/etc/passwd", pjoin(src_dir, "abs")) + os.symlink("../../../etc/passwd", pjoin(src_dir, "dotdot")) + os.symlink("sub/../../etc/passwd", pjoin(src_dir, "interleaved")) + os.symlink("..", pjoin(src_dir, "bare_dotdot")) + + with _sshfs_mount(src_dir, mnt_dir): + # Safe symlinks pass through and resolve + assert os.readlink(pjoin(mnt_dir, "safe")) == "sub/target" + assert os.readlink(pjoin(mnt_dir, "safe_dot")) == "./sub/target" + with open(pjoin(mnt_dir, "safe")) as f: + assert f.read() == "hello" + + # Dangerous: readlink returns EPERM + for name in ("abs", "dotdot", "interleaved", "bare_dotdot"): + with pytest.raises(OSError) as exc_info: + os.readlink(pjoin(mnt_dir, name)) + assert exc_info.value.errno == errno.EPERM + + # Dangerous: traversal (open/stat) also EPERM + with pytest.raises(OSError) as exc_info: + open(pjoin(mnt_dir, "abs")) + assert exc_info.value.errno == errno.EPERM + + with pytest.raises(OSError) as exc_info: + os.stat(pjoin(mnt_dir, "dotdot")) + assert exc_info.value.errno == errno.EPERM + + +def test_no_contain_symlinks(tmpdir, capfd) -> None: + """Opt-out: symlinks pass through and actually resolve.""" + + capfd.register_output(r"^Warning: Permanently added 'localhost' .+", count=0) + _check_ssh_localhost() + + mnt_dir = str(tmpdir.mkdir("mnt")) + src_dir = str(tmpdir.mkdir("src")) + + os.symlink("/etc/passwd", pjoin(src_dir, "abs_link")) + os.symlink("../../../etc/passwd", pjoin(src_dir, "rel_escape")) + + with _sshfs_mount(src_dir, mnt_dir, ["no_contain_symlinks"]): + assert os.readlink(pjoin(mnt_dir, "abs_link")) == "/etc/passwd" + assert os.readlink(pjoin(mnt_dir, "rel_escape")) == "../../../etc/passwd" + + # Absolute symlink actually resolves (reads local /etc/passwd) + with open(pjoin(mnt_dir, "abs_link")) as f: + assert "root" in f.read() + + # Relative escape: kernel must traverse the link (not EPERM). + # Target won't exist on the test host, so we just assert that + # sshfs didn't block it - any errno other than EPERM proves + # containment is genuinely disabled. + with pytest.raises(OSError) as exc_info: + os.stat(pjoin(mnt_dir, "rel_escape")) + assert exc_info.value.errno != errno.EPERM + + +def test_transform_with_contain(tmpdir, capfd) -> None: + """transform_symlinks + default containment: transformed ../x is rejected.""" + + capfd.register_output(r"^Warning: Permanently added 'localhost' .+", count=0) + capfd.register_output(r"^warning: transform_symlinks.+", count=0) + _check_ssh_localhost() + + mnt_dir = str(tmpdir.mkdir("mnt")) + src_dir = str(tmpdir.mkdir("src")) + + os.makedirs(pjoin(src_dir, "other")) + with open(pjoin(src_dir, "other", "file"), "w") as f: + f.write("data") + # Absolute in-base: transform rewrites to "other/file" (no ..) + os.symlink(pjoin(src_dir, "other", "file"), pjoin(src_dir, "inbase")) + # Absolute in-base but sibling: transform rewrites to "../other/file" + os.makedirs(pjoin(src_dir, "sub")) + os.symlink(pjoin(src_dir, "other", "file"), pjoin(src_dir, "sub", "sibling")) + + with _sshfs_mount(src_dir, mnt_dir, ["transform_symlinks"]): + # Direct child: transform produces "other/file" - no .., passes + link = os.readlink(pjoin(mnt_dir, "inbase")) + assert ".." not in link.split("/") + with open(pjoin(mnt_dir, "inbase")) as f: + assert f.read() == "data" + + # Sibling: transform produces "../other/file" - has .., EPERM + with pytest.raises(OSError) as exc_info: + os.readlink(pjoin(mnt_dir, "sub", "sibling")) + assert exc_info.value.errno == errno.EPERM + + # Same setup with no_contain_symlinks: sibling works + with _sshfs_mount(src_dir, mnt_dir, + ["transform_symlinks", "no_contain_symlinks"]): + link = os.readlink(pjoin(mnt_dir, "sub", "sibling")) + assert ".." in link + with open(pjoin(mnt_dir, "sub", "sibling")) as f: + assert f.read() == "data" + + +def test_contain_symlinks_option_precedence(tmpdir, capfd) -> None: + """Last option wins when contain_symlinks and no_contain_symlinks both set.""" + + capfd.register_output(r"^Warning: Permanently added 'localhost' .+", count=0) + _check_ssh_localhost() + + mnt_dir = str(tmpdir.mkdir("mnt")) + src_dir = str(tmpdir.mkdir("src")) + + os.symlink("/etc/passwd", pjoin(src_dir, "abs")) + + # no_contain_symlinks last: containment disabled, readlink succeeds + with _sshfs_mount(src_dir, mnt_dir, + ["contain_symlinks", "no_contain_symlinks"]): + assert os.readlink(pjoin(mnt_dir, "abs")) == "/etc/passwd" + + # contain_symlinks last: containment enabled, EPERM + with _sshfs_mount(src_dir, mnt_dir, + ["no_contain_symlinks", "contain_symlinks"]): + with pytest.raises(OSError) as exc_info: + os.readlink(pjoin(mnt_dir, "abs")) + assert exc_info.value.errno == errno.EPERM