[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#700806: marked as done (unblock: openconnect/3.20-3 (Fixes CVE-2012-6128))



Your message dated Sun, 24 Feb 2013 11:31:03 +0100
with message-id <5129EBE7.8040509@thykier.net>
and subject line Re: Bug#700806: unblock: openconnect/3.20-3 (Fixes CVE-2012-6128)
has caused the Debian Bug report #700806,
regarding unblock: openconnect/3.20-3 (Fixes CVE-2012-6128)
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
700806: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=700806
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message ---
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock

Dear Release Team,

Please unblock package openconnect, version 3.20-3 already uploaded to
unstable. This upload fixes RC bug #700794 (CVE-2012-6128), a
stack-based buffer overflow vulnerability.

The fix was made upstream and this change is a backport of that patch to
version 3.20. The debdiff is included below. Thanks in advance.


diffstat for openconnect-3.20 openconnect-3.20

 changelog                      |    7 +
 patches/02_CVE-2012-6128.patch |  281 +++++++++++++++++++++++++++++++++++++++++
 patches/series                 |    1 
 3 files changed, 289 insertions(+)

diff -Nru openconnect-3.20/debian/changelog openconnect-3.20/debian/changelog
--- openconnect-3.20/debian/changelog	2012-06-06 08:54:48.000000000 -0400
+++ openconnect-3.20/debian/changelog	2013-02-17 12:25:52.000000000 -0500
@@ -1,3 +1,10 @@
+openconnect (3.20-3) unstable; urgency=low
+
+  * debian/patches/02_CVE-2012-6128.patch: Backport patch from upstream to fix
+    buffer overflow (CVE-2012-6128). (Closes: #700794)
+
+ -- Mike Miller <mtmiller@ieee.org>  Sun, 17 Feb 2013 11:56:35 -0500
+
 openconnect (3.20-2) unstable; urgency=low
 
   * Depend on vpnc-scripts for routing and DNS configuration. (Closes:
diff -Nru openconnect-3.20/debian/patches/02_CVE-2012-6128.patch openconnect-3.20/debian/patches/02_CVE-2012-6128.patch
--- openconnect-3.20/debian/patches/02_CVE-2012-6128.patch	1969-12-31 19:00:00.000000000 -0500
+++ openconnect-3.20/debian/patches/02_CVE-2012-6128.patch	2013-02-17 12:25:52.000000000 -0500
@@ -0,0 +1,281 @@
+Origin: upstream, http://git.infradead.org/users/dwmw2/openconnect.git/commitdiff/26f752c3dbf69227679fc6bebb4ae071aecec491
+From: Kevin Cernekee <cernekee@gmail.com>
+Subject: http: Fix overflow on HTTP request buffers
+
+A malicious VPN gateway can send a very long hostname/path (for redirects)
+or cookie list (in general), which OpenConnect will attempt to sprintf()
+into a fixed length buffer.  Each HTTP server response line can add
+roughly MAX_BUF_LEN (131072) bytes to the next OpenConnect HTTP request,
+but the request buffer (buf) is capped at MAX_BUF_LEN bytes and is
+allocated on the stack.
+
+The result of passing a long "Location:" header looks like:
+
+    Attempting to connect to server 127.0.0.1:443
+    SSL negotiation with localhost
+    Server certificate verify failed: self signed certificate in certificate chain
+    Connected to HTTPS on localhost
+    GET https://localhost/
+    Got HTTP response: HTTP/1.0 301 Moved
+    Ignoring unknown HTTP response line 'aaaaaaaaaaaaaaaaaa'
+    SSL negotiation with localhost
+    Server certificate verify failed: self signed certificate in certificate chain
+    Connected to HTTPS on localhost
+    *** buffer overflow detected ***: /scr/openconnect2/.libs/lt-openconnect terminated
+    ======= Backtrace: =========
+    /lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x5c)[0x7fd62729b82c]
+    /lib/x86_64-linux-gnu/libc.so.6(+0x109700)[0x7fd62729a700]
+    /lib/x86_64-linux-gnu/libc.so.6(+0x108b69)[0x7fd627299b69]
+    /lib/x86_64-linux-gnu/libc.so.6(_IO_default_xsputn+0xdd)[0x7fd62720d13d]
+    /lib/x86_64-linux-gnu/libc.so.6(_IO_vfprintf+0x1ae7)[0x7fd6271db4a7]
+    /lib/x86_64-linux-gnu/libc.so.6(__vsprintf_chk+0x94)[0x7fd627299c04]
+    /lib/x86_64-linux-gnu/libc.so.6(__sprintf_chk+0x7d)[0x7fd627299b4d]
+    /scr/openconnect2/.libs/libopenconnect.so.2(openconnect_obtain_cookie+0xc0)[0x7fd62832d210]
+    /scr/openconnect2/.libs/lt-openconnect[0x40413f]
+    /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xed)[0x7fd6271b276d]
+    /scr/openconnect2/.libs/lt-openconnect[0x404579]
+
+The proposed fix is to use dynamically allocated buffers with overflow
+checking.
+
+--- a/http.c
++++ b/http.c
+@@ -32,6 +32,7 @@
+ #include <pwd.h>
+ #include <sys/stat.h>
+ #include <sys/types.h>
++#include <stdarg.h>
+ 
+ #include <openssl/ssl.h>
+ #include <openssl/err.h>
+@@ -45,6 +46,85 @@ static int proxy_read(struct openconnect
+ 		      unsigned char *buf, size_t len);
+ 
+ #define MAX_BUF_LEN 131072
++#define BUF_CHUNK_SIZE 4096
++
++struct oc_text_buf {
++	char *data;
++	int pos;
++	int buf_len;
++	int error;
++};
++
++static struct oc_text_buf *buf_alloc(void)
++{
++	return calloc(1, sizeof(struct oc_text_buf));
++}
++
++static void buf_append(struct oc_text_buf *buf, const char *fmt, ...)
++{
++	va_list ap;
++
++	if (!buf || buf->error)
++		return;
++
++	if (!buf->data) {
++		buf->data = malloc(BUF_CHUNK_SIZE);
++		if (!buf->data) {
++			buf->error = -ENOMEM;
++			return;
++		}
++		buf->buf_len = BUF_CHUNK_SIZE;
++	}
++
++	while (1) {
++		int max_len = buf->buf_len - buf->pos, ret;
++
++		va_start(ap, fmt);
++		ret = vsnprintf(buf->data + buf->pos, max_len, fmt, ap);
++		va_end(ap);
++		if (ret < 0) {
++			buf->error = -EIO;
++			break;
++		} else if (ret < max_len) {
++			buf->pos += ret;
++			break;
++		} else {
++			int new_buf_len = buf->buf_len + BUF_CHUNK_SIZE;
++
++			if (new_buf_len > MAX_BUF_LEN) {
++				/* probably means somebody is messing with us */
++				buf->error = -E2BIG;
++				break;
++			}
++
++			buf->data = realloc(buf->data, new_buf_len);
++			if (!buf->data) {
++				buf->error = -ENOMEM;
++				break;
++			}
++			buf->buf_len = new_buf_len;
++		}
++	}
++}
++
++static int buf_error(struct oc_text_buf *buf)
++{
++	return buf ? buf->error : -ENOMEM;
++}
++
++static int buf_free(struct oc_text_buf *buf)
++{
++	int error = buf_error(buf);
++
++	if (buf) {
++		if (buf->data)
++			free(buf->data);
++		free(buf);
++	}
++
++	return error;
++}
++
+ /*
+  * We didn't really want to have to do this for ourselves -- one might have
+  * thought that it would be available in a library somewhere. But neither
+@@ -352,7 +432,7 @@ static int fetch_config(struct openconne
+ 			char *server_sha1)
+ {
+ 	struct vpn_option *opt;
+-	char buf[MAX_BUF_LEN];
++	struct oc_text_buf *buf;
+ 	char *config_buf = NULL;
+ 	int result, buflen;
+ 	unsigned char local_sha1_bin[SHA_DIGEST_LENGTH];
+@@ -360,21 +440,26 @@ static int fetch_config(struct openconne
+ 	EVP_MD_CTX c;
+ 	int i;
+ 
+-	sprintf(buf, "GET %s%s HTTP/1.1\r\n", fu, bu);
+-	sprintf(buf + strlen(buf), "Host: %s\r\n", vpninfo->hostname);
+-	sprintf(buf + strlen(buf),  "User-Agent: %s\r\n", vpninfo->useragent);
+-	sprintf(buf + strlen(buf),  "Accept: */*\r\n");
+-	sprintf(buf + strlen(buf),  "Accept-Encoding: identity\r\n");
++	buf = buf_alloc();
++	buf_append(buf, "GET %s%s HTTP/1.1\r\n", fu, bu);
++	buf_append(buf, "Host: %s\r\n", vpninfo->hostname);
++	buf_append(buf, "User-Agent: %s\r\n", vpninfo->useragent);
++	buf_append(buf, "Accept: */*\r\n");
++	buf_append(buf, "Accept-Encoding: identity\r\n");
+ 
+ 	if (vpninfo->cookies) {
+-		sprintf(buf + strlen(buf),  "Cookie: ");
++		buf_append(buf, "Cookie: ");
+ 		for (opt = vpninfo->cookies; opt; opt = opt->next)
+-			sprintf(buf + strlen(buf),  "%s=%s%s", opt->option,
++			buf_append(buf, "%s=%s%s", opt->option,
+ 				      opt->value, opt->next ? "; " : "\r\n");
+ 	}
+-	sprintf(buf + strlen(buf),  "X-Transcend-Version: 1\r\n\r\n");
++	buf_append(buf, "X-Transcend-Version: 1\r\n\r\n");
++
++	if (buf_error(buf))
++		return buf_free(buf);
+ 
+-	SSL_write(vpninfo->https_ssl, buf, strlen(buf));
++	SSL_write(vpninfo->https_ssl, buf->data, buf->pos);
++	buf_free(buf);
+ 
+ 	buflen = process_http_response(vpninfo, &result, NULL, &config_buf);
+ 	if (buflen < 0) {
+@@ -630,7 +715,7 @@ int internal_parse_url(char *url, char *
+ int openconnect_obtain_cookie(struct openconnect_info *vpninfo)
+ {
+ 	struct vpn_option *opt, *next;
+-	char buf[MAX_BUF_LEN];
++	struct oc_text_buf *buf;
+ 	char *form_buf = NULL;
+ 	int result, buflen;
+ 	char request_body[2048];
+@@ -658,27 +743,26 @@ int openconnect_obtain_cookie(struct ope
+ 	 *
+ 	 * So we process the HTTP for ourselves...
+ 	 */
+-	sprintf(buf, "%s /%s HTTP/1.1\r\n", method, vpninfo->urlpath ?: "");
+-	sprintf(buf + strlen(buf), "Host: %s\r\n", vpninfo->hostname);
+-	sprintf(buf + strlen(buf),  "User-Agent: %s\r\n", vpninfo->useragent);
+-	sprintf(buf + strlen(buf),  "Accept: */*\r\n");
+-	sprintf(buf + strlen(buf),  "Accept-Encoding: identity\r\n");
++	buf = buf_alloc();
++	buf_append(buf, "%s /%s HTTP/1.1\r\n", method, vpninfo->urlpath ?: "");
++	buf_append(buf, "Host: %s\r\n", vpninfo->hostname);
++	buf_append(buf, "User-Agent: %s\r\n", vpninfo->useragent);
++	buf_append(buf, "Accept: */*\r\n");
++	buf_append(buf, "Accept-Encoding: identity\r\n");
+ 
+ 	if (vpninfo->cookies) {
+-		sprintf(buf + strlen(buf),  "Cookie: ");
++		buf_append(buf, "Cookie: ");
+ 		for (opt = vpninfo->cookies; opt; opt = opt->next)
+-			sprintf(buf + strlen(buf),  "%s=%s%s", opt->option,
+-				      opt->value, opt->next ? "; " : "\r\n");
++			buf_append(buf, "%s=%s%s", opt->option,
++				   opt->value, opt->next ? "; " : "\r\n");
+ 	}
+ 	if (request_body_type) {
+-		sprintf(buf + strlen(buf),  "Content-Type: %s\r\n",
+-			      request_body_type);
+-		sprintf(buf + strlen(buf),  "Content-Length: %zd\r\n",
+-			      strlen(request_body));
++		buf_append(buf, "Content-Type: %s\r\n", request_body_type);
++		buf_append(buf, "Content-Length: %zd\r\n", strlen(request_body));
+ 	}
+-	sprintf(buf + strlen(buf),  "X-Transcend-Version: 1\r\n\r\n");
++	buf_append(buf, "X-Transcend-Version: 1\r\n\r\n");
+ 	if (request_body_type)
+-		sprintf(buf + strlen(buf), "%s", request_body);
++		buf_append(buf, "%s", request_body);
+ 
+ 	if (vpninfo->port == 443)
+ 		vpn_progress(vpninfo, PRG_INFO, "%s https://%s/%s\n";,
+@@ -689,7 +773,11 @@ int openconnect_obtain_cookie(struct ope
+ 			     method, vpninfo->hostname, vpninfo->port,
+ 			     vpninfo->urlpath ?: "");
+ 
+-	result = openconnect_SSL_write(vpninfo, buf, strlen(buf));
++	if (buf_error(buf))
++		return buf_free(buf);
++
++	result = openconnect_SSL_write(vpninfo, buf->data, buf->pos);
++	buf_free(buf);
+ 	if (result < 0)
+ 		return result;
+ 
+@@ -1114,21 +1202,28 @@ static int process_socks_proxy(struct op
+ static int process_http_proxy(struct openconnect_info *vpninfo, int ssl_sock)
+ {
+ 	char buf[MAX_BUF_LEN];
++	struct oc_text_buf *reqbuf;
+ 	int buflen, result;
+ 
+-	sprintf(buf, "CONNECT %s:%d HTTP/1.1\r\n", vpninfo->hostname, vpninfo->port);
+-	sprintf(buf + strlen(buf), "Host: %s\r\n", vpninfo->hostname);
+-	sprintf(buf + strlen(buf), "User-Agent: %s\r\n", vpninfo->useragent);
+-	sprintf(buf + strlen(buf), "Proxy-Connection: keep-alive\r\n");
+-	sprintf(buf + strlen(buf), "Connection: keep-alive\r\n");
+-	sprintf(buf + strlen(buf), "Accept-Encoding: identity\r\n");
+-	sprintf(buf + strlen(buf), "\r\n");
++	reqbuf = buf_alloc();
++	buf_append(reqbuf, "CONNECT %s:%d HTTP/1.1\r\n", vpninfo->hostname, vpninfo->port);
++	buf_append(reqbuf, "Host: %s\r\n", vpninfo->hostname);
++	buf_append(reqbuf, "User-Agent: %s\r\n", vpninfo->useragent);
++	buf_append(reqbuf, "Proxy-Connection: keep-alive\r\n");
++	buf_append(reqbuf, "Connection: keep-alive\r\n");
++	buf_append(reqbuf, "Accept-Encoding: identity\r\n");
++	buf_append(reqbuf, "\r\n");
++
++	if (buf_error(reqbuf))
++		return buf_free(reqbuf);
+ 
+ 	vpn_progress(vpninfo, PRG_INFO,
+ 		     _("Requesting HTTP proxy connection to %s:%d\n"),
+ 		     vpninfo->hostname, vpninfo->port);
+ 
+-	result = proxy_write(vpninfo, ssl_sock, (unsigned char *)buf, strlen(buf));
++	result = proxy_write(vpninfo, ssl_sock, (unsigned char *)reqbuf->data, reqbuf->pos);
++	buf_free(reqbuf);
++
+ 	if (result) {
+ 		vpn_progress(vpninfo, PRG_ERR,
+ 			     _("Sending proxy request failed: %s\n"),
diff -Nru openconnect-3.20/debian/patches/series openconnect-3.20/debian/patches/series
--- openconnect-3.20/debian/patches/series	2012-06-06 08:54:48.000000000 -0400
+++ openconnect-3.20/debian/patches/series	2013-02-17 12:25:52.000000000 -0500
@@ -1 +1,2 @@
 01_man-vpnc-script-path.patch
+02_CVE-2012-6128.patch

--- End Message ---
--- Begin Message ---
On 2013-02-24 11:04, Thijs Kinkhorst wrote:
>> As mentioned in #700805, this line introduces a memory leak if realloc
>> fails for any reason.
> 
> Upstream has committed a fix for the issue but also concluded that this 
> causing real world trouble is not very probable.
> 

Personally, I am not a huge fan of "probably not an issue"-assertions in
cases like this.  If upstream is wrong on this, we will have another CVE
on our hands.

> So either the patch needs to be applied to openconnect or the package needs to 
> be unblocked as-is. Both are valid options in my opinion.
> 
> 
> Cheers,
> Thijs

With that said, I have unblocked it as it is the same fix as we have in
stable.

Mike, once openconnect/3.20-3 has migrated, you are welcome to upload a
-4 fixing this possible memory leak (actually I would appreciate it).

~Niels

--- End Message ---

Reply to: