Bug 64249

Summary: Fix read-after-free in http2 response-reader when a response is discarded
Product: Apache httpd-2 Reporter: Barnim Dzwillo <dzwillo>
Component: mod_http2Assignee: Apache HTTPD Bugs Mailing List <bugs>
Severity: normal    
Priority: P2    
Version: 2.4.41   
Target Milestone: ---   
Hardware: HP   
OS: Linux   
Attachments: Bugfix for described mod_http2 issue
Example dump from address sanitizer for described issue

Description Barnim Dzwillo 2020-03-20 17:04:28 UTC
Created attachment 37110 [details]
Bugfix for described mod_http2 issue

When the client of a multiplexed http2 connection aborts a http2 stream, and
the server side has still response-data to deliver, the cleanup process
might read from invalid data-buckets. 

In practice this ends up mostly unnoticed, because the http2 module just
discards the response-data after each line. But interesting things might
happen if another valid http status line is found in this data.

I encountered this problem, when i debugged another apache issue with the
gcc address-sanitizer enabled.

In our company we run a httpd service with http2-connections to the client
side and http-proxy-connections to the backend side. The httpd is configured
with 64 worker-threads per process, and the machines are using redhat7 linux.

The attached patch fixes the issue for our setup.

An example of how to reproduce this issue is given below.

What happens here?
- an apache module like mod_proxy_http passes a brigade with http response-data
  via the output-filter-chain to the http2 handlers (so that the response can
  be delivered to the http2-client). 
  The response-data in the brigade might be passed in form of transient-buckets.
  A transient bucket contains just a pointer to the real data located in a
  heap-bucket owned by another apache-module. And this other apache module might
  free this pointer when the scope of the output-handler is left.
  So the http2 module must copy the response-data into its own heap-buckets, when
  the data is queued for later use. This is done below in apr_bucket_setaside():
  output_filter: <h2_filter_parse_h1>
  -> h2_from_h1_parse_response(task, bb)
     -> get_line(bb); /* removes status line & headers from brigade */
     -> pass_response(h2_task *task, ap_filter_t *f, h2_response_parser *parser)
        -> ap_pass_brigade(f->next, tmp); /* pass response headers to <h2_filter_slave_output> */

  output_filter: <h2_filter_slave_output> /* handles header & body brigade */
  -> task = h2_ctx_get_task(filter->c);
     -> slave_out(task, filter, brigade);
     -> send_out(task, task->output.bb, blocking);
        -> h2_beam_send(task->output.beam, bb, block? APR_BLOCK_READ : APR_NONBLOCK_READ);
           -> while(b = APR_BRIGADE_FIRST(sender_bb))
                 append_bucket(beam, b, block, &space_left, &bl);
                 -> transient/file: apr_bucket_setaside(b, beam->send_pool); /* convert transient->heap */
                 -> heap: as-is
                 -> H2_BLIST_INSERT_TAIL(&beam->send_list, b);

- but when the http2-module processes response-status-lines or header-lines,
  and an incomplete line is read, it enqueues the transient-buckets from other
  apache modules without conversion:

  output_filter: <h2_filter_parse_h1>
  -> if (!task->output.sent_response)
        h2_from_h1_parse_response(task, bb)
        -> get_line(bb)
           -> apr_brigade_split_line(parser->tmp, bb, APR_BLOCK_READ, HUGE_STRING_LEN);
              -> moves transient buckets from bb to parser->tmp unless linefeed is found
              apr_brigade_flatten(parser->tmp, line, &len);
              if (strcmp("\r\n", line) != 0) {
                 return APR_EAGAIN; /* transient buckets stay queued in parser->tmp */

- when the output-filter-chain is called later with more response-data,
  the queued data might already be invalid, because the memory might got
  reallocated for another purpose.

- such a case occurs, when a http2-client aborts its connection. The http2 module
  seems to allocate a new task-struct for a http2 stream whenever an old task is
  done. This new task gets the remaining response-data via the output-filter-chain,
  and since task->output.sent_response is 0, it starts to scan for a new status
  line in this output. Such a response-body might be very large and might contain
  no linefeeds, so that all this data is enqueued in parser->tmp.
  (it is a problem in its own that such an amount of data is stored in parser->tmp,
  only to discard it shortly after - perhaps h2_from_h1_parse_response() shouldn't
  get called at all when task->c->aborted==1)

- in the attached dump from the address-sanitizer, the illegal read-after-free
  operation happens in the context of the ap_die_r() function, when an eos bucket
  is send down through the output-filter-chain at the end of the request-processing
  (but it might be possible to happen also earlier in the request processing):

  -> ap_die_r()
     -> ap_finalize_request_protocol(r)
        -> ap_discard_request_body(r);
           -> end_output_stream(request_rec *r)
              -> ap_pass_brigade(r->output_filters, bb); /* bucket eos only */
                 -> output_filter: <ap_content_length_filter>
                 -> output_filter: <h2_filter_headers_out>
                 -> output_filter: <h2_filter_parse_h1>
                    -> if (!task->output.sent_response)
                          h2_from_h1_parse_response(task, bb)
                          -> get_line(bb)
                             -> the content of the parser->tmp brigade is touched here

How to reproduce this issue?
1) use existing apache setup with http2 support & a working ssl-domain
2) start a simple http webserver serving content without linefeed, like:
   # socat TCP-LISTEN:9999,crlf,reuseaddr,fork SYSTEM:"echo HTTP/1.1 200 OK; echo; while [ true ]; do echo -n 0123456789 0123456789; done"
3) configure a local proxy-redirect for the test-domain in httpd.conf, like:
   RewriteRule (/http2test) http://localhost:9999/ [L,P]
4) Request mixed content from the test-domain using a multiplexed http2-connection:
   # DOM=test.de && curl -i --http2 --max-time 0.5 -o /dev/null https://$DOM/http2test -o /dev/null https://$DOM/http2test -o /dev/null https://$DOM/ -o /dev/null https://$DOM/ ...
5) Some retries or variation with the timeout and parallel http2-streams might
   be needed to trigger the issue. (The timing & memory reuse must be right)

Comment 1 Barnim Dzwillo 2020-03-20 17:05:31 UTC
Created attachment 37111 [details]
Example dump from address sanitizer for described issue
Comment 2 Barnim Dzwillo 2021-06-17 11:56:29 UTC

I can confirm that this issue was fixed by the upstream patch https://svn.apache.org/viewvc?view=revision&revision=1879546
which solves the problem in a similar way.