On Sat, 27 Dec 2014, Scott Talbert wrote:
of cookies. Increasing the length of the statically allocated buffer
for HTTP headers from 1000 to 10000 fix the issue (line 385 of
web.cpp). Tell me if you want a patch.
Thanks! This has been fixed upstream:
https://sourceforge.net/p/concordance/bugs/40/
I'll work with Matt to get the patch into Debian ASAP.
Hi Matt,
Here's a debdiff which applies the patch and has a couple other lintian
warning fixes. Can you please get it applied?
Thanks,
Scott
diff -Nru concordance-1.1/debian/changelog concordance-1.1/debian/changelog
--- concordance-1.1/debian/changelog 2014-10-07 22:57:05.000000000 -0400
+++ concordance-1.1/debian/changelog 2014-12-28 22:07:55.000000000 -0500
@@ -1,3 +1,13 @@
+concordance (1.1-3) unstable; urgency=medium
+
+ [ Scott Talbert ]
+ * Include patch from upstream to fix crash during website communications
+ (Closes: #774014)
+ * Update standards version
+ * Fix CFLAGS/CPPFLAGS for concordance so that hardening flags are used
+
+ -- Scott Talbert <s...@techie.net> Sun, 28 Dec 2014 21:27:13 -0500
+
concordance (1.1-2) unstable; urgency=medium
[ Scott Talbert ]
diff -Nru concordance-1.1/debian/control concordance-1.1/debian/control
--- concordance-1.1/debian/control 2014-10-07 22:56:28.000000000 -0400
+++ concordance-1.1/debian/control 2014-12-28 20:46:12.000000000 -0500
@@ -4,7 +4,7 @@
Homepage: http://sourceforge.net/projects/concordance
Maintainer: Mathieu Trudel-Lapierre <mathieu...@gmail.com>
Build-Depends: debhelper (>= 9), libusb-dev [hurd-any], libhidapi-dev
[linux-any kfreebsd-any], python-all-dev (>= 2.6.6-3~), pkg-config, libzip-dev,
dh-autoreconf
-Standards-Version: 3.9.5
+Standards-Version: 3.9.6
Package: concordance
Architecture: any
diff -Nru concordance-1.1/debian/patches/fix-buffer-overrun.patch
concordance-1.1/debian/patches/fix-buffer-overrun.patch
--- concordance-1.1/debian/patches/fix-buffer-overrun.patch 1969-12-31
19:00:00.000000000 -0500
+++ concordance-1.1/debian/patches/fix-buffer-overrun.patch 2014-12-28
20:44:51.000000000 -0500
@@ -0,0 +1,116 @@
+From 8d02c60a3ecfffffd7075129ce0bcbaca5558e96 Mon Sep 17 00:00:00 2001
+From: Scott Talbert <s...@techie.net>
+Date: Sat, 22 Nov 2014 12:11:39 -0500
+Subject: [PATCH] Fix buffer overrun crash in website communications
+Origin: upstream,
https://sourceforge.net/p/concordance/src/ci/8d02c60a3ecfffffd7075129ce0bcbaca5558e96/
+Bug: https://sourceforge.net/p/concordance/bugs/40/
+Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=774014
+
+Add a new format_string() function that automatically calculates buffer sizes
+and use it where appropriate instead of sprintf().
+
+Signed-off-by: Scott Talbert <s...@techie.net>
+Signed-off-by: Phil Dibowitz <p...@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
+
diff -Nru concordance-1.1/debian/patches/series
concordance-1.1/debian/patches/series
--- concordance-1.1/debian/patches/series 2014-08-24 13:51:41.000000000
-0400
+++ concordance-1.1/debian/patches/series 2014-12-28 20:45:39.000000000
-0500
@@ -1,3 +1,4 @@
conditionally_run_iptables.patch
handle_kfreebsd_hurd.patch
libzip_configure.patch
+fix-buffer-overrun.patch
diff -Nru concordance-1.1/debian/rules concordance-1.1/debian/rules
--- concordance-1.1/debian/rules 2014-08-24 13:51:41.000000000 -0400
+++ concordance-1.1/debian/rules 2014-12-28 21:56:47.000000000 -0500
@@ -28,7 +28,7 @@
override_dh_auto_configure:
( cd libconcord/ && ./configure --prefix=/usr --sysconfdir=/etc )
- ( cd concordance/ && CFLAGS=" -L../libconcord/.libs " CPPFLAGS="
-I../libconcord " ./configure --prefix=/usr )
+ ( cd concordance/ && ./configure --prefix=/usr $(shell
DEB_CFLAGS_APPEND=-L../libconcord/.libs DEB_CPPFLAGS_APPEND=-I../libconcord
dpkg-buildflags --export=configure))
build-ext-%:
( cd libconcord/bindings/python && python$* ./setup.py build )
