[prev in list] [next in list] [prev in thread] [next in thread] 

List:       bugtraq
Subject:    safely concatenating strings in portable C (Re: GnuPG 1.4 and 2.0 buffer overflow)
From:       Solar Designer <solar () openwall ! com>
Date:       2006-11-30 1:41:35
Message-ID: 20061130014135.GB1999 () openwall ! com
[Download RAW message or body]

On Mon, Nov 27, 2006 at 06:13:02PM +0100, Werner Koch wrote:
> +    n = strlen(s) + (defname?strlen (defname):0) + 10;
>      prompt = xmalloc(n);
>      if( defname )
>         sprintf(prompt, "%s [%s]: ", s, defname );
...
> Note, that using snprintf would not have helped in
> this case.  How I wish C-90 had introduced asprintf or at least it
> would be available on more platforms.

Actually, if you dare to use snprintf() (either because you don't need
your code to be portable to platforms that lack snprintf() or because
you provide an implementation along with your application), then
implementing asprintf() on top of snprintf() is fairly easy (although
you do need to consider both snprintf() return value conventions).

However, in those (most common) cases when all you need is to concatenate
strings, relying on or providing an snprintf() implementation might be
an overkill.  For that reason, I wrote a trivial concat() function (for
popa3d) that should be much easier to audit than a full-blown *printf().
Please feel free to reuse it in any applications with no need to even
credit me (consider it hereby placed in the public domain, or you may
use it under the terms of the popa3d LICENSE).

The usage of concat() is as follows:

	prompt = concat(s, " [", defname, "]: ", NULL);

You may also define an xconcat() (that would exit the program if the
allocation fails).

The code may be found here:

	http://cvsweb.openwall.com/cgi/cvsweb.cgi/Owl/packages/popa3d/popa3d/misc.c?rev=HEAD

(it's the last function currently defined in that source file), or here
is the current revision:

#include <string.h>
#include <stdarg.h>
#include <stdlib.h>
#include <limits.h>

char *concat(char *s1, ...)
{
	va_list args;
	char *s, *p, *result;
	unsigned long l, m, n;

	m = n = strlen(s1);
	va_start(args, s1);
	while ((s = va_arg(args, char *))) {
		l = strlen(s);
		if ((m += l) < l) break;
	}
	va_end(args);
	if (s || m >= INT_MAX) return NULL;

	result = malloc(m + 1);
	if (!result) return NULL;

	memcpy(p = result, s1, n);
	p += n;
	va_start(args, s1);
	while ((s = va_arg(args, char *))) {
		l = strlen(s);
		if ((n += l) < l || n > m) break;
		memcpy(p, s, l);
		p += l;
	}
	va_end(args);
	if (s || m != n || p - result != n) {
		free(result);
		return NULL;
	}

	*p = 0;
	return result;
}

-- 
Alexander Peslyak <solar at openwall.com>
GPG key ID: 5B341F15  fp: B3FB 63F4 D7A3 BCCC 6F6E  FC55 A2FC 027C 5B34 1F15
http://www.openwall.com - bringing security into open computing environments
[prev in list] [next in list] [prev in thread] [next in thread]