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