From 1269b8a686bf1254b03cd38af78167a04aa6ec88 Mon Sep 17 00:00:00 2001 From: "djm@openbsd.org" Date: Fri, 19 Mar 2021 02:18:28 +0000 Subject: upstream: 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@ OpenBSD-Commit-ID: 4e67d60d81bde7b84a742b4ee5a34001bdf80d9c --- sftp-server.c | 62 ++++++++++++++++++++++++++++++++++++----------------------- 1 file changed, 38 insertions(+), 24 deletions(-) diff --git a/sftp-server.c b/sftp-server.c index 752820f5..4cf85623 100644 --- a/sftp-server.c +++ b/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. * @@ -55,7 +55,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; @@ -748,7 +748,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; @@ -758,30 +759,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); } -- cgit v1.2.3