You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Dean Gaudet <dg...@arctic.org> on 1998/02/27 07:20:12 UTC

[PATCH] fast os_escape_path

Folks might remember a while back it was pointed out that one of the two
"beck" attacks was still a bit choking on the server, and it was because
os_escape_path is way slow.  I posted a patch that used a precomputed
table to speed things up... well here is that patch done again properly.
In this case we precompute the table at compile time.  It contains a few
other bits.

This isn't really tested well, I'm posting because: 

(a) I want to know how win32 folks are going to deal with the need to run
gen_istable at compile time. 

(b) I'm not going to bother finishing it unless folks are going to accept
it into 1.3.

Dean

Index: ap/.cvsignore
===================================================================
RCS file: /export/home/cvs/apache-1.3/src/ap/.cvsignore,v
retrieving revision 1.1
diff -u -r1.1 .cvsignore
--- .cvsignore	1997/11/15 19:04:49	1.1
+++ .cvsignore	1998/02/27 06:13:34
@@ -1 +1,3 @@
 Makefile
+gen_istable
+istable.h
Index: ap/Makefile.tmpl
===================================================================
RCS file: /export/home/cvs/apache-1.3/src/ap/Makefile.tmpl,v
retrieving revision 1.13
diff -u -r1.13 Makefile.tmpl
--- Makefile.tmpl	1998/02/22 04:37:09	1.13
+++ Makefile.tmpl	1998/02/27 06:13:34
@@ -7,7 +7,7 @@
 LIB=libap.a
 
 OBJS=ap_signal.o ap_slack.o ap_snprintf.o ap_strings.o ap_cpystrn.o \
-	ap_execve.o
+	ap_execve.o ap_istable.o
 
 .c.o:
 	$(CC) -c $(INCLUDES) $(CFLAGS) $(SPACER) $<
@@ -15,7 +15,7 @@
 all: $(LIB)
 
 clean:
-	rm -f *.o *.a
+	rm -f *.o *.a gen_istable istable.h
 
 $(OBJS): Makefile
 
@@ -23,6 +23,14 @@
 	rm -f $@
 	ar cr $@ $(OBJS)
 	$(RANLIB) $@
+
+gen_istable: gen_istable.o
+	$(CC) $(CFLAGS) $(LDFLAGS) $(LDFLAGS_SHLIB_EXPORT) -o gen_istable gen_istable.o $(LIBS)
+
+istable.h: gen_istable
+	./gen_istable >istable.h
+
+ap_istable.o: istable.h
 
 # We really don't expect end users to use this rule.  It works only with
 # gcc, and rebuilds Makefile.tmpl.  You have to re-run Configure after
Index: ap/ap_istable.c
===================================================================
RCS file: ap_istable.c
diff -N ap_istable.c
--- /dev/null	Thu Feb 26 22:13:00 1998
+++ ap_istable.c	Thu Feb 26 22:13:34 1998
@@ -0,0 +1,7 @@
+#include "ap_ctype.h"
+
+unsigned char ap_istable[256] = {
+#include "istable.h"
+};
+
+void ap_istable_is_not_here() {}
Index: ap/gen_istable.c
===================================================================
RCS file: gen_istable.c
diff -N gen_istable.c
--- /dev/null	Thu Feb 26 22:13:00 1998
+++ gen_istable.c	Thu Feb 26 22:13:34 1998
@@ -0,0 +1,63 @@
+#include <stdio.h>
+#include "ap_ctype.h"
+
+void main(int argc, char **argv)
+{
+    int c;
+    unsigned char bits;
+
+    for (c = 0; c < 256; ++c) {
+	bits = 0;
+
+	switch(c) {
+	case 'a': case 'b': case 'c': case 'd': case 'e': case 'f': case 'g':
+	case 'h': case 'i': case 'j': case 'k': case 'l': case 'm': case 'n':
+	case 'o': case 'p': case 'q': case 'r': case 's': case 't': case 'u':
+	case 'v': case 'w': case 'x': case 'y': case 'z':
+	    bits |= AP_ISLOWER | AP_ISPATHSAFE | AP_ISSCHEME;
+	    break;
+
+	case 'A': case 'B': case 'C': case 'D': case 'E': case 'F': case 'G':
+	case 'H': case 'I': case 'J': case 'K': case 'L': case 'M': case 'N':
+	case 'O': case 'P': case 'Q': case 'R': case 'S': case 'T': case 'U':
+	case 'V': case 'W': case 'X': case 'Y': case 'Z':
+	    bits |= AP_ISLOWER | AP_ISPATHSAFE | AP_ISSCHEME;
+	    break;
+
+	case '0': case '1': case '2': case '3': case '4': case '5': case '6':
+	case '7': case '8': case '9':
+	    bits |= AP_ISDIGIT | AP_ISPATHSAFE | AP_ISSCHEME;
+	    break;
+	}
+
+	switch(c) {
+	case '$': case '-': case '_': case '.': case '+': case '!': case '*':
+	case '(': case ')': case ',': case ':': case '@': case '&': case '=':
+	case '~': case '\'': case '/':
+	    bits |= AP_ISPATHSAFE;
+	    break;
+	}
+
+	switch(c) {
+	case '+': case '-': case '.':
+	    bits |= AP_ISSCHEME;
+	    break;
+	}
+	
+	switch(c) {
+	case '&': case ';': case '`': case '\'': case '\"': case '|': case '*':
+	case '?': case '~': case '<': case '>': case '^': case '(': case ')':
+	case '[': case ']': case '{': case '}': case '$': case '\\': case '\n':
+	    bits |= AP_ISSHELLUNSAFE;
+	    break;
+	}
+	printf(" 0x%02x", bits);
+	if (c != 255) {
+	    printf(",");
+	}
+	if ((c & 0xf) == 0xf) {
+	    printf("\n");
+	}
+    }
+    exit(0);
+}
Index: include/ap_ctype.h
===================================================================
RCS file: ap_ctype.h
diff -N ap_ctype.h
--- /dev/null	Thu Feb 26 22:13:00 1998
+++ ap_ctype.h	Thu Feb 26 22:13:34 1998
@@ -0,0 +1,21 @@
+#ifndef APACHE_AP_CTYPE_H
+#define APACHE_AP_CTYPE_H
+
+#define AP_ISLOWER		0x01
+#define AP_ISUPPER		0x02
+#define AP_ISDIGIT		0x04
+#define AP_ISSHELLUNSAFE	0x08	/* shell special characters */
+#define AP_ISPATHSAFE		0x10	/* path special characters */
+#define AP_ISSCHEME		0x20	/* scheme special characters */
+
+extern unsigned char ap_istable[256];
+
+#define ap_islower(x)		(ap_istable[(unsigned)(x)] & AP_ISLOWER)
+#define ap_isupper(x)		(ap_istable[(unsigned)(x)] & AP_ISUPPER)
+#define ap_isalpha(x)		(ap_istable[(unsigned)(x)] & AP_ISLOWER|AP_ISUPPER)
+#define ap_isdigit(x)		(ap_istable[(unsigned)(x)] & AP_ISDIGIT)
+#define ap_isshellunsafe(x)	(ap_istable[(unsigned)(x)] & AP_ISSHELLUNSAFE)
+#define ap_ispathsafe(x)	(ap_istable[(unsigned)(x)] & AP_ISPATHSAFE)
+#define ap_isscheme(x)		(ap_istable[(unsigned)(x)] & AP_ISSCHEME)
+
+#endif	/* !APACHE_AP_H */
Index: main/util.c
===================================================================
RCS file: /export/home/cvs/apache-1.3/src/main/util.c,v
retrieving revision 1.93
diff -u -r1.93 util.c
--- util.c	1998/02/02 22:33:34	1.93
+++ util.c	1998/02/27 06:13:39
@@ -73,6 +73,7 @@
 extern char *fgets(char *s, int, FILE*);
 extern int fclose(FILE *);
 #endif
+#include "ap_ctype.h"
 
 
 const char month_snames[12][4] =
@@ -979,7 +980,7 @@
 	}
 #endif
 
-	if (ind("&;`'\"|*?~<>^()[]{}$\\\n", cmd[x]) != -1) {
+	if (ap_isshellunsafe(cmd[x])) {
 	    for (y = l + 1; y > x; y--)
 		cmd[y] = cmd[y - 1];
 	    l++;		/* length has been increased */
@@ -1084,7 +1085,14 @@
     }
 }
 
-#define c2x(what,where) sprintf(where,"%%%02x",(unsigned char)what)
+static const char c2x_table[] = "0123456789abcdef";
+
+static ap_inline void c2x(unsigned char what, char *where)
+{
+    where[0] = '%';
+    where[1] = c2x_table[what >> 4];
+    where[2] = c2x_table[what & 0xf];
+}
 
 /*
  * escape_path_segment() escapes a path segment, as defined in RFC 1808. This
@@ -1107,13 +1115,8 @@
     char *copy = palloc(p, 3 * strlen(segment) + 1);
 
     for (x = 0, y = 0; segment[x]; x++, y++) {
-	char c = segment[x];
-#ifndef CHARSET_EBCDIC
-	if ((c < 'A' || c > 'Z') && (c < 'a' || c > 'z') && (c < '0' || c > '9')
-#else /* CHARSET_EBCDIC*/
-	if (!isalnum(c)
-#endif /*CHARSET_EBCDIC*/
-	    && ind("$-_.+!*'(),:@&=~", c) == -1) {
+	unsigned char c = segment[x];
+	if (c == '/' || !ap_ispathsafe(c)) {
 	    c2x(c, &copy[y]);
 	    y += 2;
 	}
@@ -1130,22 +1133,17 @@
     char *s = copy;
 
     if (!partial) {
-	int colon = ind(path, ':');
-	int slash = ind(path, '/');
+	const char *colon = strchr(path, ':');
+	const char *slash = strchr(path, '/');
 
-	if (colon >= 0 && (colon < slash || slash < 0)) {
+	if (colon && (!slash || colon < slash)) {
 	    *s++ = '.';
 	    *s++ = '/';
 	}
     }
     for (; *path; ++path) {
-	char c = *path;
-#ifndef CHARSET_EBCDIC
-	if ((c < 'A' || c > 'Z') && (c < 'a' || c > 'z') && (c < '0' || c > '9')
-#else /* CHARSET_EBCDIC*/
-	if (!isalnum(c)
-#endif /*CHARSET_EBCDIC*/
-	    && ind("$-_.+!*'(),:@&=/~", c) == -1) {
+	unsigned char c = *path;
+	if (!ap_ispathsafe(c)) {
 	    c2x(c, s);
 	    s += 3;
 	}




Re: [PATCH] fast os_escape_path

Posted by Ben Laurie <be...@algroup.co.uk>.
Dean Gaudet wrote:
> 
> On Fri, 27 Feb 1998, Ben Laurie wrote:
> 
> > Dean Gaudet wrote:
> > > (a) I want to know how win32 folks are going to deal with the need to run
> > > gen_istable at compile time.
> >
> > Shouldn't be a problem (in Makefile.NT). OTOH, I don't see why
> > gen_istable.c needs to be run more than once...
> 
> Martin will need a different one for bs2000... other than that it's mostly
> feasible to run it only once.

Well, OK, once per charset...

Cheers,

Ben.

-- 
Ben Laurie            |Phone: +44 (181) 735 0686|Apache Group member
Freelance Consultant  |Fax:   +44 (181) 735 0689|http://www.apache.org
and Technical Director|Email: ben@algroup.co.uk |Apache-SSL author
A.L. Digital Ltd,     |http://www.algroup.co.uk/Apache-SSL
London, England.      |"Apache: TDG" http://www.ora.com/catalog/apache

Re: [PATCH] fast os_escape_path

Posted by Dean Gaudet <dg...@arctic.org>.
On Fri, 27 Feb 1998, Ben Laurie wrote:

> Dean Gaudet wrote:
> > (a) I want to know how win32 folks are going to deal with the need to run
> > gen_istable at compile time.
> 
> Shouldn't be a problem (in Makefile.NT). OTOH, I don't see why
> gen_istable.c needs to be run more than once...

Martin will need a different one for bs2000... other than that it's mostly
feasible to run it only once.

Dean


Re: [PATCH] fast os_escape_path

Posted by Ben Laurie <be...@algroup.co.uk>.
Dean Gaudet wrote:
> (a) I want to know how win32 folks are going to deal with the need to run
> gen_istable at compile time.

Shouldn't be a problem (in Makefile.NT). OTOH, I don't see why
gen_istable.c needs to be run more than once...

Cheers,

Ben.

-- 
Ben Laurie            |Phone: +44 (181) 735 0686|Apache Group member
Freelance Consultant  |Fax:   +44 (181) 735 0689|http://www.apache.org
and Technical Director|Email: ben@algroup.co.uk |Apache-SSL author
A.L. Digital Ltd,     |http://www.algroup.co.uk/Apache-SSL
London, England.      |"Apache: TDG" http://www.ora.com/catalog/apache

Re: [PATCH] fast os_escape_path

Posted by Randy Terbush <ra...@Covalent.NET>.
Dean Gaudet <dg...@arctic.org> writes:

> Folks might remember a while back it was pointed out that one of the two
> "beck" attacks was still a bit choking on the server, and it was because
> os_escape_path is way slow.  I posted a patch that used a precomputed
> table to speed things up... well here is that patch done again properly.
> In this case we precompute the table at compile time.  It contains a few
> other bits.
> 
> This isn't really tested well, I'm posting because: 
> 
> (a) I want to know how win32 folks are going to deal with the need to run
> gen_istable at compile time. 
> 
> (b) I'm not going to bother finishing it unless folks are going to accept
> it into 1.3.
> 
> Dean

Looks like this would be a nice addition to 1.3. I would support it
and begin lobbying that we choose a freeze date for 1.3 and put the
wraps on it. ??


Re: [PATCH] fast os_escape_path

Posted by Dean Gaudet <dg...@arctic.org>.
Actually I was thinking along the lines of solving our locale problem by
coding C locale-specific routines... because that's what we need to
implement the protocol.  Only issue is that it will possibly be slower
than libc routines. 

Dean

On Fri, 27 Feb 1998, Martin Kraemer wrote:

> On Thu, Feb 26, 1998 at 10:20:12PM -0800, Dean Gaudet wrote:
> > (b) I'm not going to bother finishing it unless folks are going to accept
> > it into 1.3.
> 
> +1 on concept: looks very good! There are some more places which could
> profit from a fast (and portable) c2x routine, and the ctype stuff can
> even be extended to replace and str-to-lower() and str-to-upper()
> functions currently spread about the code.
> 
> And the ap_ctype lib even is codeset independent, thanks!
> 
>     Martin
> -- 
> | S I E M E N S |  <Ma...@mch.sni.de>  |      Siemens Nixdorf
> | ------------- |   Voice: +49-89-636-46021     |  Informationssysteme AG
> | N I X D O R F |   FAX:   +49-89-636-44994     |   81730 Munich, Germany
> ~~~~~~~~~~~~~~~~My opinions only, of course; pgp key available on request
> 


Re: [PATCH] fast os_escape_path

Posted by Martin Kraemer <Ma...@mch.sni.de>.
On Thu, Feb 26, 1998 at 10:20:12PM -0800, Dean Gaudet wrote:
> (b) I'm not going to bother finishing it unless folks are going to accept
> it into 1.3.

+1 on concept: looks very good! There are some more places which could
profit from a fast (and portable) c2x routine, and the ctype stuff can
even be extended to replace and str-to-lower() and str-to-upper()
functions currently spread about the code.

And the ap_ctype lib even is codeset independent, thanks!

    Martin
-- 
| S I E M E N S |  <Ma...@mch.sni.de>  |      Siemens Nixdorf
| ------------- |   Voice: +49-89-636-46021     |  Informationssysteme AG
| N I X D O R F |   FAX:   +49-89-636-44994     |   81730 Munich, Germany
~~~~~~~~~~~~~~~~My opinions only, of course; pgp key available on request