[PATCH xserver/hw/xfree86/parser/scan.c] potential buffer overrun

[Peter Hutterer]

On Sat, Sep 17, 2011 at 06:58:18PM +0200, vdb at picaros.org wrote:
> This patch fixes a potential buffer overrun in xf86addComment().  The 
> overrun occurs if the comment string to add doesn't start with a 
> leading '#'.  

out of interest - how do you trigger this? Presumably xf86addComment should
only be called for comments, which would require the leading '#'

Cheers,
  Peter
 
> The bug can be seen from the code assuming 
>  curlen > 0 && eol_seen == 0 && iscomment == 0 :
> 
> char *
> xf86addComment(char *cur, char *add)
> 
> <...>
> 
> 	str = add;
> ->	iscomment = 0;
> 	while (*str) {
> 	    if (*str != ' ' && *str != '\t')
> 		break;
> 	    ++str;
> 	}
> ->	iscomment = (*str == '#');
> 
>         len = strlen(add);
>         endnewline = add[len - 1] == '\n';
>         len +=  1 + iscomment + (!hasnewline) + (!endnewline) + eol_seen;
>                     ^^^^^^^^^
>         if ((str = realloc(cur, len + curlen)) == NULL)
>                 return cur;
> 
>         cur = str;
> 
>         if (eol_seen || (curlen && !hasnewline))
>                 cur[curlen++] = '\n';
> ->      if (!iscomment)
> ->              cur[curlen++] = '#';
>         strcpy(cur + curlen, add);
>         if (!endnewline)
>                 strcat(cur, "\n");
> 
> The patch below is just a bugfix and tries to stay as close as 
> possible to the original code.  See also: 
>  http://lists.x.org/archives/xorg-devel/2011-August/024775.html .
> 
> Signed-off-by: Servaas Vandenberghe <vdb at picaros.org>
> diff --git a/hw/xfree86/parser/scan.c b/hw/xfree86/parser/scan.c
> index 1cff3bc..b188503 100644
> --- a/hw/xfree86/parser/scan.c
> +++ b/hw/xfree86/parser/scan.c
> @@ -1093,7 +1093,7 @@ char *
>  xf86addComment(char *cur, char *add)
>  {
>  	char *str;
> -	int len, curlen, iscomment, hasnewline = 0, endnewline;
> +	int len, curlen, iscomment, hasnewline = 0, insnewline, endnewline;
>  
>  	if (add == NULL || add[0] == '\0')
>  		return cur;
> @@ -1108,7 +1108,6 @@ xf86addComment(char *cur, char *add)
>  		curlen = 0;
>  
>  	str = add;
> -	iscomment = 0;
>  	while (*str) {
>  	    if (*str != ' ' && *str != '\t')
>  		break;
> @@ -1118,14 +1117,23 @@ xf86addComment(char *cur, char *add)
>  
>  	len = strlen(add);
>  	endnewline = add[len - 1] == '\n';
> -	len +=  1 + iscomment + (!hasnewline) + (!endnewline) + eol_seen;
>  
> -	if ((str = realloc(cur, len + curlen)) == NULL)
> +	insnewline = eol_seen || (curlen && !hasnewline);
> +	if (insnewline)
> +		len++;
> +	if (!iscomment)
> +		len++;
> +	if (!endnewline)
> +		len++;
> +
> +	/* Allocate + 1 char for '\0' terminator. */
> +	str = realloc(cur, curlen + len + 1);
> +	if (!str)
>  		return cur;
>  
>  	cur = str;
>  
> -	if (eol_seen || (curlen && !hasnewline))
> +	if (insnewline)
>  		cur[curlen++] = '\n';
>  	if (!iscomment)
>  		cur[curlen++] = '#';
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>