On Mon, Feb 27, 2012 at 09:39:53PM +0000, Dominic Hargreaves wrote:
> Source: libapache2-mod-perl2
> Severity: normal
> Version: 2.0.5-5
> User: debian...@lists.debian.org
> Usertags: hardening-format-security hardening
> 
> With hardening flags enabled, this package FTBFS:
> 
> In file included from Pool.xs:26:0:
> /build/dom-libapache2-mod-perl2_2.0.5-5-i386-x1v_OO/libapache2-mod-perl2-2.0.5/xs/APR/Pool/APR__Pool.h:
>  In function 'mpxs_cleanup_run':
> /build/dom-libapache2-mod-perl2_2.0.5-5-i386-x1v_OO/libapache2-mod-perl2-2.0.5/xs/APR/Pool/APR__Pool.h:315:9:
>  error: format not a string literal and no format arguments 
> [-Werror=format-security]
> cc1: some warnings being treated as errors

There are three other places where a variable is used as a format
string to Perl_croak(). I'm attaching a trivial patch that fixes those.
This makes the build with -Werror=format-security succeed.

If the variable can be externally controlled by untrusted input, this
is a security problem.  The two usage warnings use constant strings so
they seem safe, but I'm afraid I can't tell whether this is the case
for ERRSV in the mpxs_cleanup_run() phase.

I'm cc'ing the modperl development list. Could somebody please look
into this? Also cc'ing the Debian security team as a heads up.

In any case, please consider the patch for 2.0.6.

Thanks for your work on mod_perl,
-- 
Niko Tyni   nt...@debian.org
>From 94bdae4a6c6e480e9b287813be2fd1eb01fd7bd3 Mon Sep 17 00:00:00 2001
From: Niko Tyni <nt...@debian.org>
Date: Fri, 9 Mar 2012 22:26:37 +0200
Subject: [PATCH] Use controlled format strings for Perl_croak()

This fixed builds with gcc -Werror=format-security.
---
 xs/APR/Pool/APR__Pool.h                     |    2 +-
 xs/Apache2/ServerUtil/Apache2__ServerUtil.h |    2 +-
 xs/Apache2/SubProcess/Apache2__SubProcess.h |    4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/xs/APR/Pool/APR__Pool.h b/xs/APR/Pool/APR__Pool.h
index 4ea8da3..058864f 100644
--- a/xs/APR/Pool/APR__Pool.h
+++ b/xs/APR/Pool/APR__Pool.h
@@ -312,7 +312,7 @@ static apr_status_t mpxs_cleanup_run(void *data)
 #endif
 
     if (SvTRUE(ERRSV)) {
-        Perl_croak(aTHX_ SvPV_nolen(ERRSV));
+        Perl_croak(aTHX_ "%s", SvPV_nolen(ERRSV));
     }
 
     /* the return value is ignored by apr_pool_destroy anyway */
diff --git a/xs/Apache2/ServerUtil/Apache2__ServerUtil.h b/xs/Apache2/ServerUtil/Apache2__ServerUtil.h
index ced1c38..c64a140 100644
--- a/xs/Apache2/ServerUtil/Apache2__ServerUtil.h
+++ b/xs/Apache2/ServerUtil/Apache2__ServerUtil.h
@@ -80,7 +80,7 @@ static apr_status_t mpxs_cleanup_run(void *data)
     }
 
     if (SvTRUE(ERRSV)) {
-        Perl_croak(aTHX_ SvPV_nolen(ERRSV));
+        Perl_croak(aTHX_ "%s", SvPV_nolen(ERRSV));
     }
 
     /* the return value is ignored by apr_pool_destroy anyway */
diff --git a/xs/Apache2/SubProcess/Apache2__SubProcess.h b/xs/Apache2/SubProcess/Apache2__SubProcess.h
index aca73a3..ae9807f 100644
--- a/xs/Apache2/SubProcess/Apache2__SubProcess.h
+++ b/xs/Apache2/SubProcess/Apache2__SubProcess.h
@@ -135,7 +135,7 @@ MP_STATIC XS(MPXS_modperl_spawn_proc_prog)
     const char *usage = "Usage: spawn_proc_prog($r, $command, [\\@argv])";
 
     if (items < 2) {
-        Perl_croak(aTHX_ usage);
+        Perl_croak(aTHX_ "%s", usage);
     }
 
     SP -= items;
@@ -156,7 +156,7 @@ MP_STATIC XS(MPXS_modperl_spawn_proc_prog)
                 av_items = len+1;
             }
             else {
-                Perl_croak(aTHX_ usage);
+                Perl_croak(aTHX_ "%s", usage);
             }
         }
 
-- 
1.7.9.1


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@perl.apache.org
For additional commands, e-mail: dev-h...@perl.apache.org

Reply via email to