httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Fritsch ...@sfritsch.de>
Subject Re: svn commit: r1205894 - in /httpd/httpd/trunk: include/util_filter.h modules/cache/mod_cache.c server/util_filter.c
Date Sun, 27 Nov 2011 16:33:41 GMT
On Thu, 24 Nov 2011, jim@apache.org wrote:

> Author: jim
> Date: Thu Nov 24 15:53:16 2011
> New Revision: 1205894
>
> URL: http://svn.apache.org/viewvc?rev=1205894&view=rev
> Log:
> Use varargs...
>
> Modified:
>    httpd/httpd/trunk/include/util_filter.h
>    httpd/httpd/trunk/modules/cache/mod_cache.c
>    httpd/httpd/trunk/server/util_filter.c
>

> Modified: httpd/httpd/trunk/server/util_filter.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_filter.c?rev=1205894&r1=1205893&r2=1205894&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/util_filter.c (original)
> +++ httpd/httpd/trunk/server/util_filter.c Thu Nov 24 15:53:16 2011
> @@ -544,17 +544,25 @@ AP_DECLARE(apr_status_t) ap_pass_brigade
>  */
> AP_DECLARE(apr_status_t) ap_pass_brigade_fchk(request_rec *r,
>                                               apr_bucket_brigade *bb,
> -                                              const char *errmsg)
> +                                              const char *fmt,
> +                                              ...)
> {
>     apr_status_t rv;
> -    if (!errmsg)
> -        errmsg = "ap_pass_brigade returned";
>
>     rv = ap_pass_brigade(r->output_filters, bb);
>     if (rv != APR_SUCCESS) {
>         if (rv != AP_FILTER_ERROR) {
> -            ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r,
> -                          "%s %d", errmsg, rv);
> +            if (!fmt)
> +                ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r,
> +                              "ap_pass_brigade returned %d", rv);
> +            else {
> +                va_list ap;
> +                const char *res;
> +                va_start(ap, fmt);
> +                res = apr_pvsprintf(r->pool, fmt, ap);
> +                va_end(ap);
> +                ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r, res, NULL);
> +            }

No, this is not right. If some caller passes arguments to 
ap_pass_brigade_fchk that may cause the result of apr_pvsprintf to contain 
a "%", you would get a format-string vulnerability. This could easily 
happen if some error message included the URL.

You must use

      ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r, "%s", res);

intead.

>             return HTTP_INTERNAL_SERVER_ERROR;
>         }
>         return AP_FILTER_ERROR;
>
>
>

Mime
View raw message