Merge pull request #361 from abhinavagarwal07/contain-symlinks

add contain_symlinks option to prevent symlink escape attacks

Author: abhinavagarwal07

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) {

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

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