You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Brian Behlendorf <br...@hyperreal.org> on 1997/07/21 07:44:54 UTC
[PATCH]: PR#748, mod_imap loop
The following fixes a bug noted in PR#748, where a reference in a mapfile
to a file above the server root will cause an infinite loop. I want one or
two sanity checks before this is implemented, but it certainly seemed to
fix the problem.
Brian
Index: mod_imap.c
===================================================================
RCS file: /export/home/cvs/apache/src/mod_imap.c,v
retrieving revision 1.25
diff -C3 -r1.25 mod_imap.c
*** mod_imap.c 1997/07/19 09:48:04 1.25
--- mod_imap.c 1997/07/21 05:40:12
***************
*** 456,461 ****
--- 456,467 ----
while ( ! strncmp(value, "../", 3) || ! strcmp(value, "..") ) {
+ if ( ! strncmp(value, "../", 3) && ! strlen(directory)) {
+ url[0] = '\0';
+ log_reason("invalid directory name in map file", r->uri, r);
+ return;
+ }
+
if (directory && (slen = strlen (directory))) {
/* for each '..', knock a directory off the end
--=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=--
"Why not?" - TL brian@organic.com - hyperreal.org - apache.org
Re: [PATCH]: PR#748, mod_imap loop
Posted by Dean Gaudet <dg...@arctic.org>.
An optimizing compiler will not make this strength reduction. I mean sure
some experimental one might, but nothing production will. Optimizing
string functions as if they are operators is usually not worth the
effort... especially when you consider that many programs replace library
routines with their own. And the ANSI C set of string primitives is
lacking some that open up many optimizations (the most common example is
"copy this string and return me a pointer to the end", or "... return the
length).
Not that mod_imap isn't filled with gems like this :)
Dean
On Mon, 21 Jul 1997, Brian Behlendorf wrote:
> At 02:25 PM 7/21/97 +0100, Ben wrote:
> >Brian Behlendorf wrote:
> >
> >> + if ( ! strncmp(value, "../", 3) && ! strlen(directory)) {
> >
> >!strlen(x) is an inefficient way of saying !*x.
>
> I'll let an optimizing compiler worry about that. !strlen(directory) is
> clearer in this situation - semantically "the 'directory' variable has no
> length".
>
> Brian
>
>
> --=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=--
> "Why not?" - TL brian@organic.com - hyperreal.org - apache.org
>
Re: [PATCH]: PR#748, mod_imap loop
Posted by Brian Behlendorf <br...@organic.com>.
At 02:25 PM 7/21/97 +0100, Ben wrote:
>Brian Behlendorf wrote:
>
>> + if ( ! strncmp(value, "../", 3) && ! strlen(directory)) {
>
>!strlen(x) is an inefficient way of saying !*x.
I'll let an optimizing compiler worry about that. !strlen(directory) is
clearer in this situation - semantically "the 'directory' variable has no
length".
Brian
--=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=--
"Why not?" - TL brian@organic.com - hyperreal.org - apache.org
Re: [PATCH]: PR#748, mod_imap loop
Posted by Ben Laurie <be...@algroup.co.uk>.
Brian Behlendorf wrote:
> + if ( ! strncmp(value, "../", 3) && ! strlen(directory)) {
!strlen(x) is an inefficient way of saying !*x.
Cheers,
Ben.
--
Ben Laurie Phone: +44 (181) 994 6435 Email:
ben@algroup.co.uk
Freelance Consultant and Fax: +44 (181) 994 6472
Technical Director URL: http://www.algroup.co.uk/Apache-SSL
A.L. Digital Ltd, Apache Group member (http://www.apache.org)
London, England. Apache-SSL author
Re: [PATCH]: PR#748, mod_imap loop
Posted by Brian Behlendorf <br...@organic.com>.
I have tested this, and it appears to be quite fine. I've committed it to
the 1.3 branch; I don't have a 1.2.2-dev branch out, so hopefully someone
can commit it to that.
Brian
At 07:46 PM 7/26/97 -0700, Dean Gaudet wrote:
>On Sun, 20 Jul 1997, Brian Behlendorf wrote:
>
>> The following fixes a bug noted in PR#748, where a reference in a mapfile
>> to a file above the server root will cause an infinite loop. I want one or
>> two sanity checks before this is implemented, but it certainly seemed to
>> fix the problem.
>
>I don't think that's the correct fix. The code right after that already
>tests the directory length. It looks like directory == NULL is a
>possibility too. This is how I think it should be fixed. I haven't tested
>this at all though.
>
>Dean
>
>Index: mod_imap.c
>===================================================================
>RCS file: /export/home/cvs/apache/src/mod_imap.c,v
>retrieving revision 1.26
>diff -u -r1.26 mod_imap.c
>--- mod_imap.c 1997/07/27 01:43:25 1.26
>+++ mod_imap.c 1997/07/27 02:45:09
>@@ -475,6 +475,10 @@
> }
>
> value += 2; /* jump over the '..' that we found in the value */
>+ } else if (directory) {
>+ url[0] = '\0';
>+ log_reason("invalid directory name in map file", r->uri, r);
>+ return;
> }
>
> if (! strncmp(value, "/../", 4) || ! strcmp(value, "/..") )
>
>
>
--=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=--
"Why not?" - TL brian@organic.com - hyperreal.org - apache.org
Re: [PATCH]: PR#748, mod_imap loop
Posted by Dean Gaudet <dg...@arctic.org>.
On Sun, 20 Jul 1997, Brian Behlendorf wrote:
> The following fixes a bug noted in PR#748, where a reference in a mapfile
> to a file above the server root will cause an infinite loop. I want one or
> two sanity checks before this is implemented, but it certainly seemed to
> fix the problem.
I don't think that's the correct fix. The code right after that already
tests the directory length. It looks like directory == NULL is a
possibility too. This is how I think it should be fixed. I haven't tested
this at all though.
Dean
Index: mod_imap.c
===================================================================
RCS file: /export/home/cvs/apache/src/mod_imap.c,v
retrieving revision 1.26
diff -u -r1.26 mod_imap.c
--- mod_imap.c 1997/07/27 01:43:25 1.26
+++ mod_imap.c 1997/07/27 02:45:09
@@ -475,6 +475,10 @@
}
value += 2; /* jump over the '..' that we found in the value */
+ } else if (directory) {
+ url[0] = '\0';
+ log_reason("invalid directory name in map file", r->uri, r);
+ return;
}
if (! strncmp(value, "/../", 4) || ! strcmp(value, "/..") )
Re: [PATCH]: PR#748, mod_imap loop
Posted by Dean Gaudet <dg...@arctic.org>.
On Mon, 21 Jul 1997, Marc Slemko wrote:
> of CVS to have it automatically run committed files through expand
Just say no to the auto-source-shredding option.
"Oops taz's disk filled up and our auto-source-shredder didn't catch
the problem properly because some obscure command doesn't report the
error!"
"Oops I didn't mean to shred your binary file!"
"Oops I didn't mean to shred your Makefile.tmpl which requires hard tabs
for certain older makes to grok!"
Ditto for auto-indentation.
Dean
Re: [PATCH]: PR#748, mod_imap loop
Posted by Marc Slemko <ma...@worldgate.com>.
While I haven't looked so I can't comment on the patch for now, let me
just say:
Hah! You used tabs.
Hmm. You know, if everything were de-tabbed you could use cool features
of CVS to have it automatically run committed files through expand. Too
bad I don't think that feature works.
...and that mod_imap now only has 502 other infinite loops left. Sigh.
On Sun, 20 Jul 1997, Brian Behlendorf wrote:
>
> The following fixes a bug noted in PR#748, where a reference in a mapfile
> to a file above the server root will cause an infinite loop. I want one or
> two sanity checks before this is implemented, but it certainly seemed to
> fix the problem.
>
> Brian
>
>
> Index: mod_imap.c
> ===================================================================
> RCS file: /export/home/cvs/apache/src/mod_imap.c,v
> retrieving revision 1.25
> diff -C3 -r1.25 mod_imap.c
> *** mod_imap.c 1997/07/19 09:48:04 1.25
> --- mod_imap.c 1997/07/21 05:40:12
> ***************
> *** 456,461 ****
> --- 456,467 ----
>
> while ( ! strncmp(value, "../", 3) || ! strcmp(value, "..") ) {
>
> + if ( ! strncmp(value, "../", 3) && ! strlen(directory)) {
> + url[0] = '\0';
> + log_reason("invalid directory name in map file", r->uri, r);
> + return;
> + }
> +
> if (directory && (slen = strlen (directory))) {
>
> /* for each '..', knock a directory off the end
>
>
> --=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=--
> "Why not?" - TL brian@organic.com - hyperreal.org - apache.org
>