From 151aa809a6110c0226ae0a27f1567e42f502ae96 Mon Sep 17 00:00:00 2001 From: Andy Green Date: Thu, 15 Sep 2016 02:36:22 +0800 Subject: [PATCH] lws_snprintf Thanks to Fabrice Gilot for reporting the problem that led to uncovering this. Due to a misunderstanding of the return value of snprintf (it is not truncated according to the max size passed in) in places relying on snprintf to truncate the length overflows are possible. This patch wraps snprintf with a new lws_snprintf() which does truncate its length to allow the buffer limiting scheme to work properly. All users should update with these fixes. In 1.7.x, there's no affected code in the library itself, just a couple on instances in the test app code. --- lib/libwebsockets.c | 19 +++++++++++++++++++ lib/libwebsockets.h | 16 +++++++++++++++- lib/private-libwebsockets.h | 2 +- test-server/test-fraggle.c | 2 +- test-server/test-ping.c | 2 +- test-server/test-server-status.c | 4 ++-- 6 files changed, 39 insertions(+), 6 deletions(-) diff --git a/lib/libwebsockets.c b/lib/libwebsockets.c index 99f8ec12..5ab49a12 100755 --- a/lib/libwebsockets.c +++ b/lib/libwebsockets.c @@ -1311,6 +1311,25 @@ lws_parse_uri(char *p, const char **prot, const char **ads, int *port, return 0; } +int +lws_snprintf(char *str, size_t size, const char *format, ...) +{ + va_list ap; + int n; + + if (!size) + return 0; + + va_start(ap, format); + n = vsnprintf(str, size, format, ap); + va_end(ap); + + if (n >= size) + return size; + + return n; +} + #ifdef LWS_NO_EXTENSIONS /* we need to provide dummy callbacks for internal exts diff --git a/lib/libwebsockets.h b/lib/libwebsockets.h index 763775a7..a330ba15 100644 --- a/lib/libwebsockets.h +++ b/lib/libwebsockets.h @@ -146,7 +146,7 @@ struct sockaddr_in; #define LWS_INVALID_FILE INVALID_HANDLE_VALUE #define LWS_O_RDONLY _O_RDONLY -#define snprintf _snprintf +#define lws_snprintf _snprintf #else /* NOT WIN32 */ #include @@ -1909,6 +1909,20 @@ lws_ext_parse_options(const struct lws_extension *ext, struct lws *wsi, LWS_VISIBLE LWS_EXTERN void lws_set_allocator(void *(*realloc)(void *ptr, size_t size)); +/** + * lws_snprintf(): lws_snprintf that truncates the returned length too + * + * \param str: destination buffer + * \param size: bytes left in destination buffer + * \param format: format string + * \param ...: args for format + * + * This lets you correctly truncate buffers by concatenating lengths, if you + * reach the limit the reported length doesn't exceed the limit. + */ +LWS_VISIBLE LWS_EXTERN int +lws_snprintf(char *str, size_t size, const char *format, ...); + #ifdef __cplusplus } #endif diff --git a/lib/private-libwebsockets.h b/lib/private-libwebsockets.h index 2a5080f2..ec3cafd8 100644 --- a/lib/private-libwebsockets.h +++ b/lib/private-libwebsockets.h @@ -91,7 +91,7 @@ #endif #ifdef LWS_HAVE__SNPRINTF -#define snprintf _snprintf +#define lws_snprintf _snprintf #endif #else /* not windows --> */ diff --git a/test-server/test-fraggle.c b/test-server/test-fraggle.c index 51b5903e..cb048ee0 100644 --- a/test-server/test-fraggle.c +++ b/test-server/test-fraggle.c @@ -346,7 +346,7 @@ int main(int argc, char **argv) struct lws_client_connect_info i; address = argv[optind]; - snprintf(ads_port, sizeof(ads_port), "%s:%u", + lws_snprintf(ads_port, sizeof(ads_port), "%s:%u", address, port & 65535); memset(&i, 0, sizeof(i)); i.context = context; diff --git a/test-server/test-ping.c b/test-server/test-ping.c index ff2d8e35..5eb59be5 100644 --- a/test-server/test-ping.c +++ b/test-server/test-ping.c @@ -450,7 +450,7 @@ int main(int argc, char **argv) /* create client websockets using dumb increment protocol */ address = argv[optind]; - snprintf(ads_port, sizeof(ads_port), "%s:%u", + lws_snprintf(ads_port, sizeof(ads_port), "%s:%u", address, port & 65535); lwsl_notice("Connecting to %s...\n", ads_port); memset(&i, 0, sizeof(i)); diff --git a/test-server/test-server-status.c b/test-server/test-server-status.c index d85897b8..50fe5bf9 100644 --- a/test-server/test-server-status.c +++ b/test-server/test-server-status.c @@ -42,7 +42,7 @@ update_status(struct lws *wsi, struct per_session_data__lws_status *pss) struct tm tm; #endif - p += snprintf(p, 512, " { %s, \"wsi\":\"%d\", \"conns\":[", + p += lws_snprintf(p, 512, " { %s, \"wsi\":\"%d\", \"conns\":[", server_info, live_wsi); /* render the list */ @@ -67,7 +67,7 @@ update_status(struct lws *wsi, struct per_session_data__lws_status *pss) if (subsequent) *p++ = ','; subsequent = 1; - p += snprintf(p, sizeof(cache) - (p - start) - 1, + p += lws_snprintf(p, sizeof(cache) - (p - start) - 1, "{\"peer\":\"%s\",\"time\":\"%s\"," "\"ua\":\"%s\"}", (*pp)->ip, date, (*pp)->user_agent);