You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jeff Trawick <tr...@attglobal.net> on 2002/05/28 16:18:52 UTC

don't try this at home

okay, do try it, but (unlike somebody last night) don't try it on daedalus

GET / HTTP/1.1
Accept: */*
Host: test
Content-Type: application/x-www-form-urlencoded
Transfer-Encoding: chunked

AAAAAAAAAAAAAAAAAAA

-- 
Jeff Trawick | trawick@attglobal.net
Born in Roswell... married an alien...

[PATCH] Re: don't try this at home

Posted by Jeff Trawick <tr...@attglobal.net>.
Jeff Trawick <tr...@attglobal.net> writes:

> okay, do try it, but (unlike somebody last night) don't try it on daedalus
> 
> GET / HTTP/1.1
> Accept: */*
> Host: test
> Content-Type: application/x-www-form-urlencoded
> Transfer-Encoding: chunked
> 
> AAAAAAAAAAAAAAAAAAA

The situation where I (still) get 200 on the GET followed by 500 for
method AAAAAAA... is when "GET /" is handled by mod_autoindex.

Isn't mod_autoindex responsible for discarding the request body?

With this change I'm getting 413 for the autoindex request:

Index: modules/generators/mod_autoindex.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/generators/mod_autoindex.c,v
retrieving revision 1.108
diff -u -r1.108 mod_autoindex.c
--- modules/generators/mod_autoindex.c	18 May 2002 04:13:12 -0000	1.108
+++ modules/generators/mod_autoindex.c	31 May 2002 13:30:52 -0000
@@ -2133,6 +2133,12 @@
     /* OK, nothing easy.  Trot out the heavy artillery... */
 
     if (allow_opts & OPT_INDEXES) {
+        int errstatus;
+
+        if ((errstatus = ap_discard_request_body(r)) != OK) {
+            return errstatus;
+        }
+
         /* KLUDGE --- make the sub_req lookups happen in the right directory.
          * Fixing this in the sub_req_lookup functions themselves is difficult,
          * and would probably break virtual includes...


-- 
Jeff Trawick | trawick@attglobal.net
Born in Roswell... married an alien...

Re: don't try this at home

Posted by Jeff Trawick <tr...@attglobal.net>.
Justin Erenkrantz <je...@apache.org> writes:

> My suggestion is as follows: if the ap_die() code is one that
> forces us to drop the connection, we don't report the recursive
> error, but instead just report this one.
> 
> So the conditional on http_request.c:121 may work as:
> 
> if (r->status != HTTP_OK && !ap_status_drops_connection(type)) {
> 
> Can you try this?  -- justin

oops, just saw your note... but you fixed it already anyway :)

Thanks!!!!!

-- 
Jeff Trawick | trawick@attglobal.net
Born in Roswell... married an alien...

Re: don't try this at home

Posted by Justin Erenkrantz <je...@apache.org>.
On Thu, May 30, 2002 at 11:02:09AM -0400, Jeff Trawick wrote:
> Running HEAD, we get a segfault when the bogus request is for a
> resource for which we aren't authorized.  Here is the traceback:

In particular, this problem is related to the fact that we have
two errors on this request: 401 and 413.  The problem is that
since 401 was reported "first," the 413 is "lost" in ap_die()
when we find the recursive error (http_request.c:122) and we
reset r->status to 401.

However, 401 isn't listed as a status code that drops the
connection (line 146).  Therefore, ap_die() calls
ap_discard_request_body again - which causes HTTP_IN to get
called again.  But, since we are 413, we're not supposed to
re-read the body.  Oops.

My suggestion is as follows: if the ap_die() code is one that
forces us to drop the connection, we don't report the recursive
error, but instead just report this one.

So the conditional on http_request.c:121 may work as:

if (r->status != HTTP_OK && !ap_status_drops_connection(type)) {

Can you try this?  -- justin

Re: don't try this at home

Posted by Justin Erenkrantz <je...@apache.org>.
On Thu, May 30, 2002 at 01:33:45PM -0700, Justin Erenkrantz wrote:
> On Thu, May 30, 2002 at 11:02:09AM -0400, Jeff Trawick wrote:
> > Running HEAD, we get a segfault when the bogus request is for a
> > resource for which we aren't authorized.  Here is the traceback:
> 
> Oh, that's not good.  Something broke after I initially fixed it.
> I'm trying to chase down what happened now.  -- justin

As an update:

My problem was related to mod_bucketeer eating the error buckets.  I
fixed that and it works for me now.  (Perhaps other output filters
need to pay attention to the error bucket as well?)

I'll try with a URL that generates a 401 now.  -- justin

Re: don't try this at home

Posted by Justin Erenkrantz <je...@apache.org>.
On Thu, May 30, 2002 at 11:02:09AM -0400, Jeff Trawick wrote:
> Running HEAD, we get a segfault when the bogus request is for a
> resource for which we aren't authorized.  Here is the traceback:

Oh, that's not good.  Something broke after I initially fixed it.
I'm trying to chase down what happened now.  -- justin

Re: don't try this at home

Posted by Jeff Trawick <tr...@attglobal.net>.
Jeff Trawick <tr...@attglobal.net> writes:

> Jeff Trawick <tr...@attglobal.net> writes:
> 
> > okay, do try it, but (unlike somebody last night) don't try it on daedalus
> > 
> > GET / HTTP/1.1
> > Accept: */*
> > Host: test
> > Content-Type: application/x-www-form-urlencoded
> > Transfer-Encoding: chunked
> > 
> > AAAAAAAAAAAAAAAAAAA
> 
> Is this test (with cr-lf added where appropriate) working consistently
> for everybody that tries?
> 
> Yesterday afternoon: 
> 
>   on one build I was getting a 200 followed by a 500 (access_log said
>   500 was for method AAAAAAAAA...)
> 
>   on another build I was getting a segfault

Running HEAD, we get a segfault when the bogus request is for a
resource for which we aren't authorized.  Here is the traceback:

#0  0x4014bd41 in __kill () from /lib/libc.so.6
#1  0x4014b9b6 in raise (sig=6) at ../sysdeps/posix/raise.c:27
#2  0x4014d0d8 in abort () at ../sysdeps/generic/abort.c:88
#3  0x80be80c in ap_log_assert (szExp=0x80e945b "readbytes > 0", 
    szFile=0x80e93e4 "http_protocol.c", nLine=957) at log.c:689
#4  0x807dc8c in ap_http_filter (f=0x81b46a8, b=0x81b3f48, 
    mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=-1)
    at http_protocol.c:957
#5  0x80cae79 in ap_get_brigade (next=0x81b46a8, bb=0x81b3f48, 
    mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=8192)
    at util_filter.c:507
#6  0x80d5125 in net_time_filter (f=0x81b3f68, b=0x81b3f48, 
    mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=8192)
    at core.c:3324
#7  0x80cae79 in ap_get_brigade (next=0x81b3f68, bb=0x81b3f48, 
    mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=8192)
    at util_filter.c:507
#8  0x807f46d in ap_get_client_block (r=0x81b3808, 
    buffer=0xbfffb5c0, bufsiz=8192)
    at http_protocol.c:1769
#9  0x807f765 in ap_discard_request_body (r=0x81b3808) at http_protocol.c:1874
#10 0x8081df9 in ap_die (type=401, r=0x81b3808) at http_request.c:153
#11 0x807eaf8 in ap_http_header_filter (f=0x81b3fb0, b=0x81b4b48)
    at http_protocol.c:1448
#12 0x80caefd in ap_pass_brigade (next=0x81b3fb0, bb=0x81b4b48)
    at util_filter.c:534
#13 0x80ce3aa in ap_content_length_filter (f=0x81b3f98, b=0x81b4b48)
    at protocol.c:1292
#14 0x80caefd in ap_pass_brigade (next=0x81b3f98, bb=0x81b4b48)
    at util_filter.c:534
#15 0x8081223 in ap_byterange_filter (f=0x81b3f80, bb=0x81b4b48)
    at http_protocol.c:2745
#16 0x80caefd in ap_pass_brigade (next=0x81b3f80, bb=0x81b4b48)
    at util_filter.c:534
#17 0x807d94c in ap_http_filter (f=0x81b46a8, b=0x81b3f48, 
    mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=8192)
    at http_protocol.c:888
#18 0x80cae79 in ap_get_brigade (next=0x81b46a8, bb=0x81b3f48, 
    mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=8192)
    at util_filter.c:507
#19 0x80d5125 in net_time_filter (f=0x81b3f68, b=0x81b3f48, 
    mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=8192)
    at core.c:3324
#20 0x80cae79 in ap_get_brigade (next=0x81b3f68, bb=0x81b3f48, 
    mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=8192)
    at util_filter.c:507
#21 0x807f46d in ap_get_client_block (r=0x81b3808, 
    buffer=0xbfffd878, bufsiz=8192) at http_protocol.c:1769
#22 0x807f765 in ap_discard_request_body (r=0x81b3808) at http_protocol.c:1874
#23 0x8081df9 in ap_die (type=401, r=0x81b3808) at http_request.c:153
#24 0x80820c2 in ap_process_request (r=0x81b3808) at http_request.c:277
#25 0x807bbe1 in ap_process_http_connection (c=0x81ad848) at http_core.c:291
#26 0x80c7ce0 in ap_run_process_connection (c=0x81ad848) at connection.c:85
#27 0x80c8145 in ap_process_connection (c=0x81ad848, csd=0x81ad788)
    at connection.c:207
#28 0x80b80a4 in child_main (child_num_arg=4) at prefork.c:671
#29 0x80b8259 in make_child (s=0x8158c50, slot=4) at prefork.c:765
#30 0x80b82f2 in startup_children (number_to_start=1) at prefork.c:783
#31 0x80b87f1 in ap_mpm_run (_pconf=0x810cfe8, plog=0x814d0e8, s=0x8158c50)
    at prefork.c:999
#32 0x80bfe7c in main (argc=3, argv=0xbffffa84) at main.c:646
(gdb) 

cute recursion, huh?

I'll try to see what is up with the weird 200-followed-by-500
problem.

-- 
Jeff Trawick | trawick@attglobal.net
Born in Roswell... married an alien...

Re: don't try this at home

Posted by Allard Hoeve <al...@byte.nl>.
On 30 May 2002, Jeff Trawick wrote:

> > GET / HTTP/1.1
> > Accept: */*
> > Host: test
> > Content-Type: application/x-www-form-urlencoded
> > Transfer-Encoding: chunked
> >
> > AAAAAAAAAAAAAAAAAAA
>
> Yesterday afternoon:
>
>   on one build I was getting a 200 followed by a 500 (access_log said
>   500 was for method AAAAAAAAA...)
>
>   on another build I was getting a segfault


Dear Jeff,

I tried it on my server at my home webserver and the server (child)
segfaults with an entry in error_log:

  [Fri May 31 19:52:12 2002] [error] [client 127.0.0.1] Directory index
  forbidden by rule: /usr/local/apache/www/
  [Fri May 31 19:52:19 2002] [notice] child pid 17945 exit signal
  Segmentation fault (11)

My configuration is listed below. Mail me privately for more information
if you need it.

Cheers,

Allard Hoeve




-- 

Self-compiled on Linux 2.4.19-pre6, Debian, gcc 2.95.4,
ld version 2.12.90.0.1

Server: Apache/1.3.20 (Unix) mod_ssl/2.8.4 OpenSSL/0.9.6b mod_perl/1.26
PHP/4.1.2

Compiled-in modules:
  http_core.c
  mod_env.c
  mod_log_config.c
  mod_mime.c
  mod_negotiation.c
  mod_status.c
  mod_include.c
  mod_autoindex.c
  mod_dir.c
  mod_cgi.c
  mod_asis.c
  mod_imap.c
  mod_actions.c
  mod_userdir.c
  mod_alias.c
  mod_rewrite.c
  mod_access.c
  mod_auth.c
  mod_proxy.c
  mod_so.c
  mod_setenvif.c
suexec: enabled; valid wrapper /usr/local/httpd/bin/suexec


Re: don't try this at home

Posted by Jeff Trawick <tr...@attglobal.net>.
Jeff Trawick <tr...@attglobal.net> writes:

> okay, do try it, but (unlike somebody last night) don't try it on daedalus
> 
> GET / HTTP/1.1
> Accept: */*
> Host: test
> Content-Type: application/x-www-form-urlencoded
> Transfer-Encoding: chunked
> 
> AAAAAAAAAAAAAAAAAAA

Is this test (with cr-lf added where appropriate) working consistently
for everybody that tries?

Yesterday afternoon: 

  on one build I was getting a 200 followed by a 500 (access_log said
  500 was for method AAAAAAAAA...)

  on another build I was getting a segfault

I'll try to understand this more later today.

-- 
Jeff Trawick | trawick@attglobal.net
Born in Roswell... married an alien...

Re: don't try this at home

Posted by Aaron Bannert <aa...@clove.org>.
On Wed, May 29, 2002 at 02:47:35PM +0200, Martin Kraemer wrote:
> But IMO we need to have a way to parse the hex string and detect an
> integer overflow at the same time. If an overflow occurs, then
> an 4XX message is appropriate (400 Bad Request  rather than
> 413 Request Entity Too Large)

I mostly agree on the codes (not that it matters that much if it's
400 or 413, but I'm sure Roy has an opinion on this). I would think
that 400 makes sense for overflow, but then again, if we can't
handle the size it's not really a bad request...

> Then, as a second step (if the number parsed all right, even if it
> was incredibly long, as in this chunk of 33 bytes:
>  000000000000000000000000000000000000000000000000000000021 CRLF
> ) we can try and verify whether we accept the size. For that, we
> have an upper limit defined by "LimitRequestBody bytes".
> Anything beyond that can impossibly be accepted.

With this I completely agree with, but I think this is already
happening. I'd need to review the code to be sure.

Thanks for the leading-zeros hint, I'll fix that momentarily.

-aaron

Re: don't try this at home

Posted by Martin Kraemer <Ma...@Fujitsu-Siemens.com>.
On Tue, May 28, 2002 at 08:00:16AM -0700, Justin Erenkrantz wrote:
> On Tue, May 28, 2002 at 10:18:52AM -0400, Jeff Trawick wrote:
> > okay, do try it, but (unlike somebody last night) don't try it on daedalus
> > 
> > GET / HTTP/1.1
> > Accept: */*
> > Host: test
> > Content-Type: application/x-www-form-urlencoded
> > Transfer-Encoding: chunked
> > 
> > AAAAAAAAAAAAAAAAAAA
> 
> Hmm.  Isn't that legal?  A is a hex digit.

RFC2616:
       Chunked-Body   = *chunk
			last-chunk
			trailer
			CRLF

       chunk          = chunk-size [ chunk-extension ] CRLF
			chunk-data CRLF
       chunk-size     = 1*HEX
       last-chunk     = 1*("0") [ chunk-extension ] CRLF

so, strictly spoken, it is "legal". The trailing chunk could have been
 0000000000000000000000000000000000000000000000000000000000000 CRLF
and still be legal.

But IMO we need to have a way to parse the hex string and detect an
integer overflow at the same time. If an overflow occurs, then
an 4XX message is appropriate (400 Bad Request  rather than
413 Request Entity Too Large)

Then, as a second step (if the number parsed all right, even if it
was incredibly long, as in this chunk of 33 bytes:
 000000000000000000000000000000000000000000000000000000021 CRLF
) we can try and verify whether we accept the size. For that, we
have an upper limit defined by "LimitRequestBody bytes".
Anything beyond that can impossibly be accepted.

   Martin
-- 
<Ma...@Fujitsu-Siemens.com>         |     Fujitsu Siemens
Fon: +49-89-636-46021, FAX: +49-89-636-47655 | 81730  Munich,  Germany

Re: don't try this at home

Posted by Justin Erenkrantz <je...@apache.org>.
On Tue, May 28, 2002 at 10:37:51AM -0700, Justin Erenkrantz wrote:
> I'll take a pass at figuring out how to return this.  -- justin

It's trivial to detect this case - ctx->remaining is < 0.  But,
it is actually way harder than it should be because we call
ap_discard_body on the error handling path via ap_die().  That
means we become re-entrant to HTTP_IN.  Uh-oh.

If we were to remove HTTP_IN, when ap_discard_body() is called,
the core input filters will then block waiting for 8k of data to
be read - which is bogus as well.

So, what's the best way for an input filter to kill the request
and return an error page?  ISTR some discussion about the fact
that there is a difference between filter return codes and HTTP
error codes.

Thoughts?  Off to lunch while I ponder this some more.  -- justin

Re: don't try this at home

Posted by Justin Erenkrantz <je...@apache.org>.
On Tue, May 28, 2002 at 01:22:34PM -0400, Greg Ames wrote:
> Jeff said something about the chunk size being negative.  Clearly that's wrong,
> but while we're at it, we should review the RFC and see what the rules are.

Obviously, this is an overflow condition.

We should be returning 413:

10.4.14 413 Request Entity Too Large

   The server is refusing to process a request because the request
   entity is larger than the server is willing or able to process. The
   server MAY close the connection to prevent the client from continuing
   the request.

   If the condition is temporary, the server SHOULD include a Retry-
   After header field to indicate that it is temporary and after what
   time the client MAY try again.

I'll take a pass at figuring out how to return this.  -- justin

Re: don't try this at home

Posted by Greg Ames <gr...@apache.org>.
Justin Erenkrantz wrote:
> 
> On Tue, May 28, 2002 at 10:18:52AM -0400, Jeff Trawick wrote:
> > okay, do try it, but (unlike somebody last night) don't try it on daedalus
> >
> > GET / HTTP/1.1
> > Accept: */*
> > Host: test
> > Content-Type: application/x-www-form-urlencoded
> > Transfer-Encoding: chunked
> >
> > AAAAAAAAAAAAAAAAAAA
> 
> Hmm.  Isn't that legal?  A is a hex digit.
> 
> What should we be doing?  -- justin

well, not this for sure:

(from /usr/local/apache2.0.37dev/corefiles/httpd.core.1)

#0  0x2815e7b4 in kill () from /usr/lib/libc.so.4
#1  0x2819eb26 in abort () from /usr/lib/libc.so.4
#2  0x80699d2 in ap_log_assert (szExp=0x80854db "readbytes > 0",
    szFile=0x8085464 "http_protocol.c", nLine=911) at log.c:691
#3  0x805ffe8 in ap_http_filter (f=0x814bef8, b=0x814b798,
    mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=-1431655766)
    at http_protocol.c:911
#4  0x80717fc in ap_get_brigade (next=0x814bef8, bb=0x814b798,
    mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=8192)
    at util_filter.c:507
#5  0x8078f4d in net_time_filter (f=0x814b7b8, b=0x814b798,
    mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=8192)
    at core.c:3324
#6  0x80717fc in ap_get_brigade (next=0x814b7b8, bb=0x814b798,
    mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=8192)
    at util_filter.c:507
#7  0x8061225 in ap_get_client_block (r=0x814b048,
    buffer=0xbfbfd790 ".0-doc/config/ajp.html\n", bufsiz=8192)
    at http_protocol.c:1705
#8  0x8061407 in ap_discard_request_body (r=0x814b048) at http_protocol.c:1801
#9  0x8063153 in ap_die (type=302, r=0x814b048) at http_request.c:150
#10 0x8063376 in ap_process_request (r=0x814b048) at http_request.c:274
#11 0x805e8f3 in ap_process_http_connection (c=0x8134120) at http_core.c:291

Jeff said something about the chunk size being negative.  Clearly that's wrong,
but while we're at it, we should review the RFC and see what the rules are.

Greg

Re: don't try this at home

Posted by Jeff Trawick <tr...@attglobal.net>.
Justin Erenkrantz <je...@apache.org> writes:

> On Tue, May 28, 2002 at 10:18:52AM -0400, Jeff Trawick wrote:
> > okay, do try it, but (unlike somebody last night) don't try it on daedalus
> > 
> > GET / HTTP/1.1
> > Accept: */*
> > Host: test
> > Content-Type: application/x-www-form-urlencoded
> > Transfer-Encoding: chunked
> > 
> > AAAAAAAAAAAAAAAAAAA
> 
> Hmm.  Isn't that legal?  A is a hex digit.
> 
> What should we be doing?  -- justin

Here is what we shouldn't be doing:
/usr/local/apache/corefiles/httpd.core.1

My guess is that we have an overflow or sign problem handling the
chunk size.  Another little detail is that we puked before reading any
data following the chunk size.

-- 
Jeff Trawick | trawick@attglobal.net
Born in Roswell... married an alien...

Re: don't try this at home

Posted by Justin Erenkrantz <je...@apache.org>.
On Tue, May 28, 2002 at 10:18:52AM -0400, Jeff Trawick wrote:
> okay, do try it, but (unlike somebody last night) don't try it on daedalus
> 
> GET / HTTP/1.1
> Accept: */*
> Host: test
> Content-Type: application/x-www-form-urlencoded
> Transfer-Encoding: chunked
> 
> AAAAAAAAAAAAAAAAAAA

Hmm.  Isn't that legal?  A is a hex digit.

What should we be doing?  -- justin