Merge pull request #349 from abhinavagarwal07/fix-sftp-read-buffer-safety

fix malformed SFTP reply handling

Author: h4sh5

sshfs.c

@@ -1441,23 +1441,24 @@ static int do_read(struct conn *conn, struct buffer *buf)
 
 static int sftp_read(struct conn *conn, uint8_t *type, struct buffer *buf)
 {
-	int res;
+	int res = -1;
 	struct buffer buf2;
 	uint32_t len;
 	buf_init(&buf2, 5);
-	res = do_read(conn, &buf2);
-	if (res != -1) {
-		if (buf_get_uint32(&buf2, &len) == -1)
-			return -1;
-		if (len > MAX_REPLY_LEN) {
-			fprintf(stderr, "reply len too large: %u\n", len);
-			return -1;
-		}
-		if (buf_get_uint8(&buf2, type) == -1)
-			return -1;
-		buf_init(buf, len - 1);
-		res = do_read(conn, buf);
+	if (do_read(conn, &buf2) == -1)
+		goto out;
+	if (buf_get_uint32(&buf2, &len) == -1)
+		goto out;
+	if (len < 1 || len > MAX_REPLY_LEN) {
+		fprintf(stderr, "bad reply len: %u\n", len);
+		goto out;
 	}
+	if (buf_get_uint8(&buf2, type) == -1)
+		goto out;
+	buf_init(buf, len - 1);
+	res = do_read(conn, buf);
+
+out:
 	buf_free(&buf2);
 	return res;
 }
@@ -1530,10 +1531,14 @@ static int process_one_request(struct conn *conn)
 
 	buf_init(&buf, 0);
 	res = sftp_read(conn, &type, &buf);
-	if (res == -1)
+	if (res == -1) {
+		buf_free(&buf);
 		return -1;
-	if (buf_get_uint32(&buf, &id) == -1)
+	}
+	if (buf_get_uint32(&buf, &id) == -1) {
+		buf_free(&buf);
 		return -1;
+	}
 
 	pthread_mutex_lock(&sshfs.lock);
 	req = (struct request *)

test/test_sshfs.py

@@ -851,3 +851,44 @@ def test_direct_io(tmpdir, capfd):
         raise
     else:
         umount(mount_process, mnt_dir)
+
+
+def test_bad_sftp_reply_len(tmpdir):
+    """sshfs must reject a zero-length SFTP reply instead of underflowing."""
+    helper = tmpdir.join("bad_sftp.py")
+    helper.write(
+        '#!/usr/bin/env python3\n'
+        'import os, struct, sys\n'
+        'def read_pkt():\n'
+        '    hdr = os.read(0, 4)\n'
+        '    if len(hdr) < 4: sys.exit(0)\n'
+        '    n = struct.unpack(">I", hdr)[0]\n'
+        '    while n:\n'
+        '        c = os.read(0, n)\n'
+        '        if not c: sys.exit(0)\n'
+        '        n -= len(c)\n'
+        'read_pkt()\n'
+        'os.write(1, struct.pack(">IBI", 5, 2, 3))\n'  # SSH_FXP_VERSION v3
+        'read_pkt()\n'
+        'os.write(1, struct.pack(">IB", 0, 0))\n'  # len=0 reply (5 bytes on wire)
+    )
+    helper.chmod(0o755)
+
+    mnt_dir = str(tmpdir.mkdir("mnt"))
+    cmdline = base_cmdline + [
+        pjoin(basename, "sshfs"),
+        "-f",
+        "dummy:/",
+        mnt_dir,
+        "-o", f"ssh_command={helper}",
+    ]
+    res = subprocess.run(
+        cmdline,
+        stdin=subprocess.DEVNULL,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE,
+        timeout=10,
+        text=True,
+    )
+    assert res.returncode != 0
+    assert "bad reply len: 0" in res.stderr