httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <traw...@gmail.com>
Subject Re: svn commit: r1639717 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number modules/aaa/mod_authnz_fcgi.c
Date Fri, 14 Nov 2014 22:25:13 GMT
On Fri, Nov 14, 2014 at 1:18 PM, <ylavic@apache.org> 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/

Mime
View raw message