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 99f8ec1..5ab49a1 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 763775a..a330ba1 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 2a5080f..ec3cafd 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 51b5903..cb048ee 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 ff2d8e3..5eb59be 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 d85897b..50fe5bf 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);