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.

diff --git a/lib/libwebsockets.c b/lib/libwebsockets.c
index 99f8ec1269adc287f6ab66ce48df1ebc4a3e5f74..5ab49a1293655909409187bd1fe75182c3e6d455 100755 (executable)
--- 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 763775a7e92b65470d335a9877ce42dfb983e555..a330ba154774cb8d4d6f079d56d5e7f898062f35 100644 (file)
--- 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 <unistd.h>
@@ -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 2a5080f22ca92d189b0f89d469fda4b48ed4110d..ec3cafd83699f44fdfb34791ea635a6d3efd07f2 100644 (file)
--- 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 51b5903e441adb1c86b2bc74ca517b3450a344ec..cb048ee0410eb893ee20611aec9e5b3a14b3685b 100644 (file)
--- 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 ff2d8e3549e716da56c5e4dbd09321ac1e422e08..5eb59be50e2323c99030a9b25ed129e0cfea63f4 100644 (file)
--- 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 d85897b83bf6298b1e5658ed0fb59ae463995786..50fe5bf98552191615b24d4983ca4b9b1fe5d936 100644 (file)
--- 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);