httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ben Laurie <...@gonzo.ben.algroup.co.uk>
Subject Re: bug in directory indexing code (apache version 1.0.0-1.0.5)
Date Thu, 25 Apr 1996 13:06:31 GMT
Tony Sanders wrote:
> 
> I've been having some problems with apache 1.0.0-1.0.5
> occassionaly getting into a tight spin-loop eating up cpu.  I
> finally traced it down to the directory indexing code trashing
> the stack with a null byte.  Someone should check 1.1b* for
> this bug as well.
> 
> A patch follows -- though whoever "owns" that code might want to
> solve the problem in a different way as I was not totally clear on
> why the code was doing things the way it was doing them so I opted
> to just preserve the behavior.  I also reduced the number of times
> that the constant "23" was used -- it should probably be a #define.

Erk. I fixed this in 1.1 ... but... your patch is not correct.

This is what 1.1 says:

		char buff[24]="                       ";
		t2 = escape_html(scratch, t);
		buff[23-len] = '\0';
		t2 = pstrcat(scratch, t2, "</A>", buff, NULL);

As I said at the time, I'm a little surprised that the construct

char buff[24]="                       ";

works at all but it does seem to. Perhaps it would be more portably replaced
with:

	char buff[SOME_CONST];
	memset(buff,' ',SOME_CONST-1);
	buff[SOME_CONST-1]='\0';

> 
> *** mod_dir.c.orig	Wed Apr 24 12:45:48 1996
> --- mod_dir.c	Wed Apr 24 13:12:47 1996
> ***************
> *** 617,625 ****
>   		t2 = pstrcat(scratch, t2, "</A>", NULL);
>               } else 
>   	    {
> ! 		char buff[23]="                       ";
>   		t2 = escape_html(scratch, t);
> ! 		buff[23-len] = '\0';
>   		t2 = pstrcat(scratch, t2, "</A>", buff, NULL);
>   	    }
>   	    anchor = pstrcat (scratch, "<A HREF=\"",
> --- 617,626 ----
>   		t2 = pstrcat(scratch, t2, "</A>", NULL);
>               } else 
>   	    {
> ! 		char buff[23];
> ! 		strncpy(buff, "                       ", sizeof(buff));

This will leave buff with no terminating NUL (which may be OK).

>   		t2 = escape_html(scratch, t);
> ! 		buff[sizeof(buff)-len] = '\0';

And this comes out the same as before, so still corrupts the stack (if len ==
0).

>   		t2 = pstrcat(scratch, t2, "</A>", buff, NULL);
>   	    }
>   	    anchor = pstrcat (scratch, "<A HREF=\"",

Cheers,

Ben.

-- 
Ben Laurie                  Phone: +44 (181) 994 6435
Freelance Consultant and    Fax:   +44 (181) 994 6472
Technical Director          Email: ben@algroup.co.uk
A.L. Digital Ltd,           URL: http://www.algroup.co.uk
London, England.

Mime
View raw message