bugtraq - OpenSSL SSL_get_shared_ciphers() off-by-one buffer overflow

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <20070927162140.GA10706@fugu1.local>
Date: Thu, 27 Sep 2007 18:21:40 +0200
From: Moritz Jodeit <moritz@...eit.org>
To: bugtraq@...urityfocus.com
Subject: OpenSSL SSL_get_shared_ciphers() off-by-one buffer overflow

-----------------------------------------------------------------
OpenSSL SSL_get_shared_ciphers() off-by-one buffer overflow

Copyright (c) 2007 Moritz Jodeit <moritz@...eit.org> (2007/09/27)
-----------------------------------------------------------------

Application details:

	OpenSSL is a widely used open source implementation of the
	SSL v2/v3 and TLS v1 protocols.

Vulnerability description:

	OpenSSL 0.9.7l and 0.9.8d fixed a buffer overflow found in
	the SSL_get_shared_ciphers() function reported by Tavis
	Ormandy and Will Drewry of the Google Security Team.

	Although this fix prevented the unlimited overflow of the
	buffer, it still allowed an off-by-one buffer overflow to
	happen, which could potentially still result in remote code
	execution.

	Here is an excerpt of the function from ssl/ssl_lib.c:

	p=buf;
	sk=s->session->ciphers;
	for (i=0; i<sk_SSL_CIPHER_num(sk); i++)
	        {
	        /* Decrement for either the ':' or a '\0' */
	        len--;						[4]
	        c=sk_SSL_CIPHER_value(sk,i);
	        for (cp=c->name; *cp; )
	                {
	                if (len-- <= 0)				[1]
	                        {
	                        *p='\0';			[5]
	                        return(buf);
	                        }
	                else
	                        *(p++)= *(cp++);		[2]
	                }
	        *(p++)=':';					[3]
	        }
	p[-1]='\0';
	return(buf);

	The old vulnerability got fixed at [1] by comparing 'len'
	against <= 0 instead of == 0 to detect the possible
	underflow of 'len'.

	To trigger the off-by-one, you'd just fill the buffer
	with cipher strings up to the point, where 'len' == 1 and
	'cp' pointing to the last character of the current cipher
	string. The last round of the inner for() loop would then
	decrement 'len' to 0 at [1] and write the last byte of the
	current cipher string into the buffer [2], increasing 'p'
	to point to the last free byte of the buffer.
	The last free byte is then filled by the ':' separator and
	'p' is increased to point one byte behind the buffer.
	Now if there are still ciphers remaining, we enter the
	outer loop again, decrease 'len' to -1 at [4] and then
	hit the check at [1] again. This time it's true and the
	terminating '\0' byte is written one byte behind the
	buffer [5] before returning.

Vendor response:

	2007/06/06	Initial contact with openssl-security@...nssl.org
	2007/07/06	Response received by Ben Laurie <ben@...ks.org>
			regarding a proposed fix.
	2007/09/19	Fix committed to the OpenSSL_0_9_8-stable branch
			in CVS.

Vulnerable packages:

	All applications using the SSL_get_shared_ciphers() function from
	the OpenSSL library up to 0.9.7m and 0.9.8e.