increase maximum SSH2_FXP_READ to match the maximum packet size.

Also handle zero-length reads that are borderline nonsensical but
not explicitly banned by the spec.
Based on patch from Mike Frysinger, feedback deraadt@ ok dtucker@

diff --git a/usr.bin/ssh/sftp-server.c b/usr.bin/ssh/sftp-server.c
index da053cd..9a3d673 100644 (file)
--- a/usr.bin/ssh/sftp-server.c
+++ b/usr.bin/ssh/sftp-server.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sftp-server.c,v 1.123 2021/03/16 06:15:43 djm Exp $ */
+/* $OpenBSD: sftp-server.c,v 1.124 2021/03/19 02:18:28 djm Exp $ */
 /*
  * Copyright (c) 2000-2004 Markus Friedl.  All rights reserved.
  *
@@ -47,7 +47,7 @@
 char *sftp_realpath(const char *, char *); /* sftp-realpath.c */
 
 /* Maximum data read that we are willing to accept */
-#define SFTP_MAX_READ_LENGTH (64 * 1024)
+#define SFTP_MAX_READ_LENGTH (SFTP_MAX_MSG_LENGTH - 1024)
 
 /* Our verbosity */
 static LogLevel log_level = SYSLOG_LEVEL_ERROR;
@@ -740,7 +740,8 @@ process_close(u_int32_t id)
 static void
 process_read(u_int32_t id)
 {
-       u_char buf[SFTP_MAX_READ_LENGTH];
+       static u_char *buf;
+       static size_t buflen;
        u_int32_t len;
        int r, handle, fd, ret, status = SSH2_FX_FAILURE;
        u_int64_t off;
@@ -750,30 +751,43 @@ process_read(u_int32_t id)
            (r = sshbuf_get_u32(iqueue, &len)) != 0)
                fatal_fr(r, "parse");
 
-       debug("request %u: read \"%s\" (handle %d) off %llu len %d",
+       debug("request %u: read \"%s\" (handle %d) off %llu len %u",
            id, handle_to_name(handle), handle, (unsigned long long)off, len);
-       if (len > sizeof buf) {
-               len = sizeof buf;
-               debug2("read change len %d", len);
+       if ((fd = handle_to_fd(handle)) == -1)
+               goto out;
+       if (len > SFTP_MAX_READ_LENGTH) {
+               debug2("read change len %u to %u", len, SFTP_MAX_READ_LENGTH);
+               len = SFTP_MAX_READ_LENGTH;
        }
-       fd = handle_to_fd(handle);
-       if (fd >= 0) {
-               if (lseek(fd, off, SEEK_SET) == -1) {
-                       error("process_read: seek failed");
-                       status = errno_to_portable(errno);
-               } else {
-                       ret = read(fd, buf, len);
-                       if (ret == -1) {
-                               status = errno_to_portable(errno);
-                       } else if (ret == 0) {
-                               status = SSH2_FX_EOF;
-                       } else {
-                               send_data(id, buf, ret);
-                               status = SSH2_FX_OK;
-                               handle_update_read(handle, ret);
-                       }
-               }
+       if (len > buflen) {
+               debug3_f("allocate %zu => %u", buflen, len);
+               if ((buf = realloc(NULL, len)) == NULL)
+                       fatal_f("realloc failed");
+               buflen = len;
        }
+       if (lseek(fd, off, SEEK_SET) == -1) {
+               status = errno_to_portable(errno);
+               error_f("seek \"%.100s\": %s", handle_to_name(handle),
+                   strerror(errno));
+               goto out;
+       }
+       if (len == 0) {
+               /* weird, but not strictly disallowed */
+               ret = 0;
+       } else if ((ret = read(fd, buf, len)) == -1) {
+               status = errno_to_portable(errno);
+               error_f("read \"%.100s\": %s", handle_to_name(handle),
+                   strerror(errno));
+               goto out;
+       } else if (ret == 0) {
+               status = SSH2_FX_EOF;
+               goto out;
+       }
+       send_data(id, buf, ret);
+       handle_update_read(handle, ret);
+       /* success */
+       status = SSH2_FX_OK;
+ out:
        if (status != SSH2_FX_OK)
                send_status(id, status);
 }