[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#618857: apache2-mpm-itk: if you do not assign a user ID, the default one from Apache is _NOT_ used.



tags 618857 + patch
thanks

On Sat, Mar 19, 2011 at 01:05:43AM +0100, Samuel Montosa wrote:
> As far I tested, versions prior to 'squeeze', apache/itk behavior was as
> claimed at http://mpm-itk.sesse.net/
> 
> "
> AssignUserID: Takes two parameters, uid and gid (or really, user name
> and group name); specifies what uid and gid the vhost will run as (after
> parsing the request etc., of course).
> 
> _________Note that if you do not assign a user ID, the default one from
> Apache will be used._____________
> "
> 
> On 'squeeze', if user ID is not assigned by AssignUserID at VirtualHost,
> default ID will be __root__. User and Group directives from Apache will
> be ignored.

Hi,

I managed to reproduce your bug; it only happens if you do not set
AssignUserID but do set NiceValue. In other words, the default configuration
is unaffected (and most normal ones), but it is still an issue.

I have a patch for this, but as upstream I believe I need to go through the
CVE procedure. Does anyone from the security team (Cc-ed) want to help me
through the process? I guess first of all I need a CVE number assigned that I
can refer to in the upstream changelog.

FWIW, the patch is:

diff -ur orig/httpd-2.2.17/server/mpm/experimental/itk/itk.c httpd-2.2.17/server/mpm/experimental/itk/itk.c
--- orig/httpd-2.2.17/server/mpm/experimental/itk/itk.c 2011-03-20 13:18:18.000000000 +0100
+++ httpd-2.2.17/server/mpm/experimental/itk/itk.c      2011-03-20 13:15:42.000000000 +0100
@@ -1697,8 +1697,8 @@
 /* == merge the parent per-dir config structure into ours == */
 static void *itk_merge_dir_config(apr_pool_t *p, void *parent_ptr, void *child_ptr)
 {
-    itk_per_dir_conf *c = (itk_per_dir_conf *)
-        apr_pcalloc(p, sizeof(itk_per_dir_conf));
+    itk_per_dir_conf *c = (itk_per_dir_conf *)
+        itk_create_dir_config(p, NULL);
     itk_per_dir_conf *parent = (itk_per_dir_conf *) parent_ptr;
     itk_per_dir_conf *child = (itk_per_dir_conf *) child_ptr;

Testing would be appreciated. I'm attaching a debdiff with the patch put into
the patch system, for testing.

/* Steinar */
-- 
Homepage: http://www.sesse.net/
diff -u apache2-2.2.16/debian/changelog apache2-2.2.16/debian/changelog
--- apache2-2.2.16/debian/changelog
+++ apache2-2.2.16/debian/changelog
@@ -1,3 +1,11 @@
+apache2 (2.2.16-6+squeeze1) unstable; urgency=low
+
+  * Fix an mpm-itk security issue where sites would be run as root if
+    NiceValue was set but AssignUserID was not set, due to incorrect
+    config merging. (Closes: #618857)
+
+ -- Steinar H. Gunderson <sesse@debian.org>  Sun, 20 Mar 2011 13:33:08 +0100
+
 apache2 (2.2.16-6) unstable; urgency=low
 
   * Also add $named to the secondary-init-script example.
diff -u apache2-2.2.16/debian/mpm-itk/patches/series apache2-2.2.16/debian/mpm-itk/patches/series
--- apache2-2.2.16/debian/mpm-itk/patches/series
+++ apache2-2.2.16/debian/mpm-itk/patches/series
@@ -10,0 +11 @@
+11-fix-cve-something.patch
only in patch2:
unchanged:
--- apache2-2.2.16.orig/debian/mpm-itk/patches/11-fix-cve-something.patch
+++ apache2-2.2.16/debian/mpm-itk/patches/11-fix-cve-something.patch
@@ -0,0 +1,12 @@
+--- httpd-2.2.11.orig/server/mpm/experimental/itk/itk.c	2009-04-14 23:29:16.000000000 +0200
++++ httpd-2.2.11/server/mpm/experimental/itk/itk.c	2009-04-14 23:31:05.000000000 +0200
+@@ -1697,8 +1697,8 @@
+ /* == merge the parent per-dir config structure into ours == */
+ static void *itk_merge_dir_config(apr_pool_t *p, void *parent_ptr, void *child_ptr)
+ {
+-    itk_per_dir_conf *c = (itk_per_dir_conf *)
+-        apr_pcalloc(p, sizeof(itk_per_dir_conf));
++    itk_per_dir_conf *c = (itk_per_dir_conf *)
++        itk_create_dir_config(p, NULL);
+     itk_per_dir_conf *parent = (itk_per_dir_conf *) parent_ptr;
+     itk_per_dir_conf *child = (itk_per_dir_conf *) child_ptr;

Reply to: