Blob Blame History Raw
From 8d02c60a3ecfffffd7075129ce0bcbaca5558e96 Mon Sep 17 00:00:00 2001
From: Scott Talbert <swt@techie.net>
Date: Sat, 22 Nov 2014 12:11:39 -0500
Subject: [PATCH] Fix buffer overrun crash in website communications

Add a new format_string() function that automatically calculates buffer sizes
and use it where appropriate instead of sprintf().

Signed-off-by: Scott Talbert <swt@techie.net>
Signed-off-by: Phil Dibowitz <phil@ipom.com>
---
 libconcord/web.cpp | 56 ++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 40 insertions(+), 16 deletions(-)

diff --git a/libconcord/web.cpp b/libconcord/web.cpp
index 0a67a71..7910563 100644
--- a/libconcord/web.cpp
+++ b/libconcord/web.cpp
@@ -19,6 +19,7 @@
  * (C) Copyright Phil Dibowitz 2008
  */
 
+#include <stdarg.h>
 #include <string.h>
 #include "libconcord.h"
 #include "lc_internal.h"
@@ -62,6 +63,28 @@ static const uint8_t urlencodemap[32]={
     0xFF, 0xFF, 0xFF, 0xFF
 };
 
+/*
+ * This function does C-style string formatting, but uses a C++ string and
+ * automatically handles buffer sizing.  It is intended to be used in place of
+ * sprintf()/snprintf() where we don't necessarily know the required buffer
+ * size in advance.  The formatted string is appended to the supplied C++
+ * string.
+ */
+void format_string(string *str, const char *format, ...)
+{
+    va_list args;
+    va_start(args, format);
+    int size = vsnprintf(NULL, 0, format, args);
+    va_end(args);
+    size++; // Add room for \0
+    char *buffer = new char[size];
+    va_start(args, format);
+    vsnprintf(buffer, size, format, args);
+    va_end(args);
+    *str += buffer;
+    delete[] buffer;
+}
+
 static void UrlEncode(const char *in, string &out)
 {
     out = "";
@@ -351,25 +374,26 @@ int Post(uint8_t *xml, uint32_t xml_size, const char *root, TRemoteInfo &ri,
 
     string post;
     if (learn_seq == NULL) {
-        char serial[144];
-        sprintf(serial, "%s%s%s", ri.serial1, ri.serial2, ri.serial3);
-        char post_data[4000]; // large enough for extra usbnet headers
+        string serial;
+        format_string(&serial, "%s%s%s", ri.serial1, ri.serial2, ri.serial3);
+        string post_data;
         if (z_post) {
-            sprintf(post_data, z_post_xml, ri.hw_ver_major, ri.hw_ver_minor,
-                    ri.flash_mfg, ri.flash_id, ri.fw_ver_major,
+            format_string(&post_data, z_post_xml, ri.hw_ver_major,
+                    ri.hw_ver_minor, ri.flash_mfg, ri.flash_id, ri.fw_ver_major,
                     ri.fw_ver_minor);
         } else {
-            sprintf(post_data, post_xml, ri.fw_ver_major, ri.fw_ver_minor,
-                    ri.fw_type, serial, ri.hw_ver_major, ri.hw_ver_minor,
-                    ri.hw_ver_micro, ri.flash_mfg, ri.flash_id, ri.protocol,
-                    ri.architecture, ri.skin);
-            sprintf(post_data+strlen(post_data), "%s", post_xml_trailer);
+            format_string(&post_data, post_xml, ri.fw_ver_major,
+                    ri.fw_ver_minor, ri.fw_type, serial.c_str(),
+                    ri.hw_ver_major, ri.hw_ver_minor, ri.hw_ver_micro,
+                    ri.flash_mfg, ri.flash_id, ri.protocol, ri.architecture,
+                    ri.skin);
+            format_string(&post_data, "%s", post_xml_trailer);
         }
 
-        debug("post data: %s",post_data);
+        debug("post data: %s", post_data.c_str());
 
         string post_data_encoded;
-        UrlEncode(post_data, post_data_encoded);
+        UrlEncode(post_data.c_str(), post_data_encoded);
 
         post = "Data=" + post_data_encoded;
     } else {
@@ -382,11 +406,11 @@ int Post(uint8_t *xml, uint32_t xml_size, const char *root, TRemoteInfo &ri,
 
     debug("%s", post.c_str());
 
-    char http_header[1000];
-    sprintf(http_header, post_header, path.c_str(), server.c_str(),
+    string http_header;
+    format_string(&http_header, post_header, path.c_str(), server.c_str(),
             cookie.c_str(), post.length());
 
-    debug("%s", http_header);
+    debug("%s", http_header.c_str());
 
-    return Zap(server, http_header,post.c_str());
+    return Zap(server, http_header.c_str(), post.c_str());
 }
-- 
2.1.0