From: Damien Miller Subject: sshbuf_get_nulterminated_string() To: tech@openbsd.org Cc: openssh@openssh.com Date: Thu, 20 Nov 2025 11:42:55 +1100 Hi, This adds a sshbuf_get_nulterminated_string() to OpenSSH's sshbuf code to extract a null-terminated string from a buffer. Mostly sshbuf is used to work with the RFC4253 SSH wire encoding, but there are a few places where we need to interact with data that is formatted using other rules. One example is the dynamic "-D" forwarding code, where we need to parse SOCKS headers, and these do use some nul-terminated strings. I'd like to rewrite the SOCKS code to use the safe sshbuf for parsing instead of its current manual string-fiddling. We also need to support incremental parsing as ssh's channel layer may not have received a complete SOCKS header. This is supported by this function returning SSH_ERR_MESSAGE_INCOMPLETE when there wasn't a nul-terminated string of the maximum desired length in the buffer. ok? Index: usr.bin/ssh/sshbuf-getput-basic.c =================================================================== RCS file: /cvs/src/usr.bin/ssh/sshbuf-getput-basic.c,v diff -u -p -r1.13 sshbuf-getput-basic.c --- usr.bin/ssh/sshbuf-getput-basic.c 25 May 2022 06:03:44 -0000 1.13 +++ usr.bin/ssh/sshbuf-getput-basic.c 20 Nov 2025 00:34:41 -0000 @@ -627,3 +627,36 @@ sshbuf_get_bignum2_bytes_direct(struct s } return 0; } + +int +sshbuf_get_nulterminated_string(struct sshbuf *buf, size_t maxlen, + char **valp, size_t *lenp) +{ + u_char zero = 0; + const char *val = NULL; + size_t len = 0; + int r; + + if (valp != NULL) + *valp = NULL; + if (lenp != NULL) + *lenp = 0; + if ((r = sshbuf_find(buf, 0, &zero, sizeof(zero), &len)) != 0) { + if (r == SSH_ERR_INVALID_FORMAT && sshbuf_len(buf) < maxlen) + return SSH_ERR_MESSAGE_INCOMPLETE; + return r; + } + if (len > maxlen) + return SSH_ERR_INVALID_FORMAT; + /* can strdup() because it's definitely nul-terminated */ + if ((val = strdup(sshbuf_ptr(buf))) == NULL) + return SSH_ERR_ALLOC_FAIL; + if ((r = sshbuf_consume(buf, len + 1)) != 0) { + free(val); + return r; + } + /* success */ + *valp = val; + *lenp = len; + return 0; +} Index: usr.bin/ssh/sshbuf.h =================================================================== RCS file: /cvs/src/usr.bin/ssh/sshbuf.h,v diff -u -p -r1.32 sshbuf.h --- usr.bin/ssh/sshbuf.h 2 Sep 2025 09:41:23 -0000 1.32 +++ usr.bin/ssh/sshbuf.h 20 Nov 2025 00:34:41 -0000 @@ -231,6 +231,10 @@ int sshbuf_put_ec(struct sshbuf *buf, co int sshbuf_put_eckey(struct sshbuf *buf, const EC_KEY *v); int sshbuf_put_ec_pkey(struct sshbuf *buf, EVP_PKEY *pkey); +/* Functions to extract or store various non-SSH wire encoded values */ +int sshbuf_get_nulterminated_string(struct sshbuf *buf, size_t maxlen, + char **valp, size_t *lenp); + /* Dump the contents of the buffer in a human-readable format */ void sshbuf_dump(const struct sshbuf *buf, FILE *f); Index: regress/usr.bin/ssh/unittests/sshbuf/test_sshbuf_getput_basic.c =================================================================== RCS file: /cvs/src/regress/usr.bin/ssh/unittests/sshbuf/test_sshbuf_getput_basic.c,v diff -u -p -r1.5 test_sshbuf_getput_basic.c --- regress/usr.bin/ssh/unittests/sshbuf/test_sshbuf_getput_basic.c 15 Sep 2025 03:00:22 -0000 1.5 +++ regress/usr.bin/ssh/unittests/sshbuf/test_sshbuf_getput_basic.c 20 Nov 2025 00:34:41 -0000 @@ -710,4 +710,53 @@ sshbuf_getput_basic_tests(void) sshbuf_free(p1); free(s2); TEST_DONE(); + + TEST_START("sshbuf_get_nulterminated_string"); + p1 = sshbuf_new(); + ASSERT_INT_EQ(sshbuf_put(p1, "hello", 5), 0); + ASSERT_INT_EQ(sshbuf_put_u8(p1, 0), 0); + ASSERT_INT_EQ(sshbuf_put(p1, "there", 5), 0); + ASSERT_SIZE_T_EQ(sshbuf_len(p1), 11); + /* short maxlen */ + ASSERT_INT_EQ(sshbuf_get_nulterminated_string(p1, 1, &s2, &s), + SSH_ERR_INVALID_FORMAT); + ASSERT_INT_EQ(sshbuf_get_nulterminated_string(p1, 4, &s2, &s), + SSH_ERR_INVALID_FORMAT); + ASSERT_PTR_EQ(s2, NULL); + ASSERT_SIZE_T_EQ(s, 0); + ASSERT_SIZE_T_EQ(sshbuf_len(p1), 11); + /* minimim usable maxlen */ + ASSERT_INT_EQ(sshbuf_get_nulterminated_string(p1, 5, &s2, &s), 0); + ASSERT_STRING_EQ(s2, "hello"); + ASSERT_SIZE_T_EQ(s, 5); + ASSERT_SIZE_T_EQ(sshbuf_len(p1), 5); + free(s2); + /* final string is not nul-terminated */ + ASSERT_INT_EQ(sshbuf_get_nulterminated_string(p1, 5, &s2, &s), + SSH_ERR_INVALID_FORMAT); + ASSERT_INT_EQ(sshbuf_get_nulterminated_string(p1, 6, &s2, &s), + SSH_ERR_MESSAGE_INCOMPLETE); + ASSERT_INT_EQ(sshbuf_get_nulterminated_string(p1, SIZE_MAX, &s2, &s), + SSH_ERR_MESSAGE_INCOMPLETE); + ASSERT_PTR_EQ(s2, NULL); + ASSERT_SIZE_T_EQ(s, 0); + /* prepare for round 2 */ + ASSERT_INT_EQ(sshbuf_put_u8(p1, 0), 0); + ASSERT_INT_EQ(sshbuf_put(p1, "it is", 5), 0); + ASSERT_SIZE_T_EQ(sshbuf_len(p1), 11); + /* maxlen = string length + 1 */ + ASSERT_INT_EQ(sshbuf_get_nulterminated_string(p1, 6, &s2, &s), 0); + ASSERT_STRING_EQ(s2, "there"); + ASSERT_SIZE_T_EQ(s, 5); + ASSERT_SIZE_T_EQ(sshbuf_len(p1), 5); + free(s2); + /* maxlen > buffer len */ + ASSERT_INT_EQ(sshbuf_put_u8(p1, 0), 0); + ASSERT_INT_EQ(sshbuf_get_nulterminated_string(p1, SIZE_MAX, &s2, &s), 0); + ASSERT_STRING_EQ(s2, "it is"); + ASSERT_SIZE_T_EQ(s, 5); + ASSERT_SIZE_T_EQ(sshbuf_len(p1), 0); + free(s2); + sshbuf_free(p1); + TEST_DONE(); }