On Thu, 24 Nov 2011, j...@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;



Reply via email to