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

@@ -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

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

lib/private-libwebsockets.h

@@ -91,7 +91,7 @@
 #endif
 
 #ifdef LWS_HAVE__SNPRINTF
-#define snprintf _snprintf
+#define lws_snprintf _snprintf
 #endif
 
 #else /* not windows --> */

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;

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));

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);