From ebf644fb6a6e609dd05a5cb4c070e74ac3e85322 Mon Sep 17 00:00:00 2001 From: Viktor Szakats Date: Wed, 3 May 2023 21:17:01 +0000 Subject: [PATCH] src: fix `libssh2_store_*()` for >u32 inputs `_libssh2_store_str()` and `_libssh2_store_bignum2_bytes()` accept inputs of `size_t` max, store the size as 32-bit unsigned integer, then store the complete input buffer. With inputs larger than `UINT_MAX` this means the stored size is smaller than the data that follows it. This patch truncates the stored data to the stored size, and now returns a boolean with false if the stored length differs from the requested one. Also add `assert()`s for this condition. This is still not a correct fix, as we now dump consistent, but still truncated data which is not what the caller wants. In future steps we'll need to update all callers that might pass large data to this function to check the return value and handle an error, or make sure to not call this function with more than UINT_MAX bytes of data. Ref: c3bcdd88a44c4636818407aeb894fabc90bb0ecd (2010-04-17) Ref: ed439a29bb0b4d1c3f681f87ccfcd3e5a66c3ba0 (2022-09-29) Closes #1025 --- src/misc.c | 40 +++++++++++++++++++++++++++------------- src/misc.h | 8 ++++---- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/misc.c b/src/misc.c index 62497197..7de6511b 100644 --- a/src/misc.c +++ b/src/misc.c @@ -45,6 +45,7 @@ #endif #include +#include #ifdef WIN32 /* Force parameter type. */ @@ -254,37 +255,50 @@ void _libssh2_store_u32(unsigned char **buf, uint32_t value) /* _libssh2_store_str */ -void _libssh2_store_str(unsigned char **buf, const char *str, size_t len) +int _libssh2_store_str(unsigned char **buf, const char *str, size_t len) { - _libssh2_store_u32(buf, (uint32_t)len); - if(len) { - memcpy(*buf, str, len); - *buf += len; + uint32_t len_stored = (uint32_t)len; + + _libssh2_store_u32(buf, len_stored); + if(len_stored) { + memcpy(*buf, str, len_stored); + *buf += len_stored; } + + assert(len_stored == len); + return len_stored == len; } /* _libssh2_store_bignum2_bytes */ -void _libssh2_store_bignum2_bytes(unsigned char **buf, - const unsigned char *bytes, - size_t len) +int _libssh2_store_bignum2_bytes(unsigned char **buf, + const unsigned char *bytes, + size_t len) { - int extraByte = 0; + uint32_t len_stored; + uint32_t extraByte; const unsigned char *p; + for(p = bytes; len > 0 && *p == 0; --len, ++p) {} extraByte = (len > 0 && (p[0] & 0x80) != 0); - _libssh2_store_u32(buf, (uint32_t)(len + extraByte)); + len_stored = (uint32_t)len; + if(extraByte && len_stored == 0xffffffff) + len_stored--; + _libssh2_store_u32(buf, len_stored + extraByte); if(extraByte) { *buf[0] = 0; *buf += 1; } - if(len > 0) { - memcpy(*buf, p, len); - *buf += len; + if(len_stored) { + memcpy(*buf, p, len_stored); + *buf += len_stored; } + + assert(len_stored == len); + return len_stored == len; } /* Base64 Conversion */ diff --git a/src/misc.h b/src/misc.h index d8a26c87..95db7142 100644 --- a/src/misc.h +++ b/src/misc.h @@ -108,10 +108,10 @@ uint32_t _libssh2_ntohu32(const unsigned char *buf); libssh2_uint64_t _libssh2_ntohu64(const unsigned char *buf); void _libssh2_htonu32(unsigned char *buf, uint32_t val); void _libssh2_store_u32(unsigned char **buf, uint32_t value); -void _libssh2_store_str(unsigned char **buf, const char *str, size_t len); -void _libssh2_store_bignum2_bytes(unsigned char **buf, - const unsigned char *bytes, - size_t len); +int _libssh2_store_str(unsigned char **buf, const char *str, size_t len); +int _libssh2_store_bignum2_bytes(unsigned char **buf, + const unsigned char *bytes, + size_t len); void *_libssh2_calloc(LIBSSH2_SESSION *session, size_t size); struct string_buf *_libssh2_string_buf_new(LIBSSH2_SESSION *session);