On Fri, Nov 14, 2014 at 1:18 PM, <[email protected]> wrote:
> Author: ylavic
> Date: Fri Nov 14 18:18:15 2014
> New Revision: 1639717
>
> URL: http://svn.apache.org/r1639717
> Log:
> mod_authnz_fcgi: Fix a potential crash with response headers' size above
> 8K.
> (similar to r1638818 for mod_proxy_fcgi).
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/docs/log-message-tags/next-number
> httpd/httpd/trunk/modules/aaa/mod_authnz_fcgi.c
>
> Modified: httpd/httpd/trunk/CHANGES
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1639717&r1=1639716&r2=1639717&view=diff
>
> ==============================================================================
> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
> +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Nov 14 18:18:15 2014
> @@ -5,6 +5,9 @@ Changes with Apache 2.5.0
> mod_proxy_fcgi: Fix a potential crash with response headers' size
> above 8K.
> [Teguh <chain rop.io>, Yann Ylavic]
>
> + *) mod_authnz_fcgi: Fix a potential crash with response headers' size
> above 8K.
> + [Yann Ylavic]
> +
> *) mod_authnz_ldap: Resolve crashes with LDAP authz and non-LDAP authn
> since
> r1608202. [Eric Covener]
>
>
> Modified: httpd/httpd/trunk/docs/log-message-tags/next-number
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/log-message-tags/next-number?rev=1639717&r1=1639716&r2=1639717&view=diff
>
> ==============================================================================
> --- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
> +++ httpd/httpd/trunk/docs/log-message-tags/next-number Fri Nov 14
> 18:18:15 2014
> @@ -1 +1 @@
> -2821
> +2822
>
> Modified: httpd/httpd/trunk/modules/aaa/mod_authnz_fcgi.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/aaa/mod_authnz_fcgi.c?rev=1639717&r1=1639716&r2=1639717&view=diff
>
> ==============================================================================
> --- httpd/httpd/trunk/modules/aaa/mod_authnz_fcgi.c (original)
> +++ httpd/httpd/trunk/modules/aaa/mod_authnz_fcgi.c Fri Nov 14 18:18:15
> 2014
> @@ -406,13 +406,12 @@ enum {
> *
> * Returns 0 if it can't find the end of the headers, and 1 if it found
> the
> * end of the headers. */
> -static int handle_headers(request_rec *r,
> - int *state,
> - char *readbuf)
> +static int handle_headers(request_rec *r, int *state,
> + char *readbuf, apr_size_t readlen)
> {
> const char *itr = readbuf;
>
> - while (*itr) {
> + while (readlen) {
> if (*itr == '\r') {
> switch (*state) {
> case HDR_STATE_GOT_CRLF:
> @@ -443,13 +442,17 @@ static int handle_headers(request_rec *r
> break;
> }
> }
> - else {
> + else if (*itr == '\t' || !apr_iscntrl(*itr)) {
> *state = HDR_STATE_READING_HEADERS;
> }
> + else {
> + return -1;
> + }
>
First, thanks for taking care of this issue.
I guess I don't understand the importance of this code which returns an
error on non-tab control characters. Why isn't the character checking the
same as in ap_scan_script_header_err_brigade_ex() since that is what will
finally parse the buffer?
Thanks!
> if (*state == HDR_STATE_DONE_WITH_HEADERS)
> break;
>
> + --readlen;
> ++itr;
> }
>
> @@ -555,7 +558,17 @@ static apr_status_t handle_response(cons
> APR_BRIGADE_INSERT_TAIL(ob, b);
>
> if (!seen_end_of_headers) {
> - int st = handle_headers(r, &header_state, readbuf);
> + int st = handle_headers(r, &header_state, readbuf,
> + readbuflen);
> +
> + if (st == -1) {
> + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
> + APLOGNO(02821) "%s: error reading "
> + "headers from %s",
> + fn, conf->backend);
> + rv = APR_EINVAL;
> + break;
> + }
>
> if (st == 1) {
> int status;
> @@ -646,7 +659,7 @@ static apr_status_t handle_response(cons
> /*
> * Read/discard any trailing padding.
> */
> - if (plen) {
> + if (rv == APR_SUCCESS && plen) {
> rv = recv_data_full(conf, r, s, readbuf, plen);
> if (rv != APR_SUCCESS) {
> ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r,
>
>
>
--
Born in Roswell... married an alien...
http://emptyhammock.com/
