[PATCH] sftp: check the argument to -B for validity

[jelly]

The -B option is parsed with strtol() and the result assigned to a size_t
without any range checking beyond rejecting zero and trailing garbage.
Negative values therefore wrap around to a huge size_t (-1 becomes
SIZE_MAX), and values above the SFTP protocol limit are accepted as well;
the value is truncated to u_int and handed to sftp_init(), where it is
used to size the transfer buffers - "-1" ends up trying to xmalloc()
about 4GB.

Parse the argument with strtonum() and reject values outside the range
that can actually be transmitted: the buffer is used to build
SSH2_FXP_WRITE requests, whose payload cannot exceed SFTP_MAX_MSG_LENGTH,
so the usable maximum is SFTP_MAX_MSG_LENGTH - 1024 (the same slack
sftp-server reserves for SFTP_MAX_READ_LENGTH). Larger values previously
failed mid-transfer with "Outbound message too long"; document the bound
in sftp.1.
---
sftp.1 |  2 ++
sftp.c | 13 ++++++++++---
2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/sftp.1 b/sftp.1
index a30338ba5..dab4878a3 100644
--- a/sftp.1
+++ b/sftp.1
@@ -122,6 +122,8 @@ uses when transferring files.
Larger buffers require fewer round trips at the cost of higher
memory consumption.
The default is 32768 bytes.
+The maximum supported value is 261120 bytes (256 KB less room for the
+SFTP message header).
.It Fl b Ar batchfile
Batch mode reads a series of commands from an input
.Ar batchfile
diff --git a/sftp.c b/sftp.c
index ecbaefd61..e46bd6e57 100644
--- a/sftp.c
+++ b/sftp.c
@@ -2586,9 +2586,16 @@ main(int argc, char **argv)
			global_aflag = 1;
			break;
		case 'B':
-			copy_buffer_len = strtol(optarg, &cp, 10);
-			if (copy_buffer_len == 0 || *cp != '\0')
-				fatal("Invalid buffer size \"%s\"", optarg);
+			/*
+			 * The transfer buffer is used to build SSH2_FXP_WRITE
+			 * requests, so leave room for the message header.
+			 */
+			llv = strtonum(optarg, 1, SFTP_MAX_MSG_LENGTH - 1024,
+			    &errstr);
+			if (errstr != NULL)
+				fatal("Invalid buffer size \"%s\": %s", optarg,
+				    errstr);
+			copy_buffer_len = (size_t)llv;
			break;
		case 'b':
			if (batchmode)
--
2.43.0