You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Colm MacCárthaigh <co...@Redbrick.DCU.IE> on 2002/05/26 05:17:48 UTC

suexec related patches for PR 7810, 7791, 8291, 9038

Since there have been some changes to the affected source files
and multiple problems presented themselves in unixd.c, my patches
to make suexec + [ mod_include | mod_userdir | mod_cgid ] work
were getting stale. So I've rediffed them against CVS.

I also had a good look through all of the suexec bugs, I'm using
the patches on a production system now with over 2000 shell users
(redbrick.dcu.ie) and it's proving stable.

Anyway, I think they fix these :

 PR 7810 - suexec + mod_userdir + mod_cgid needed fixing (also 
           it's currently insecure by default, this really needs
           to be fixed)
 PR 7791 - malformed arguments array passed to suexec
 PR 8291 - mod_include + suexec "exec cmd" not working
 PR 9038 - really a duplicate of 7810

Some notes:

  1: http://redbrick.dcu.ie/~colmmacc/patches/mod_cgid.patch
  2: http://redbrick.dcu.ie/~colmmacc/patches/unixd.patch
  3: http://redbrick.dcu.ie/~colmmacc/patches/mod_include.patch

  patch 1 (mod_cgid.c)    fixes 7810/9039/mod_cgid, it just works.
  patch 2 (unixd.c)       fixes 7791 and 8291 
  patch 3 (mod_include.c) makes patch 2 secure. (otherwise include
                          file="some.cgi" runs as the server user)
Other Patches:

These are against 2.0.36, but should apply to CVS.   

Whilst trawling code for patch 2 I noticed that in 
srclib/apr/threadproc/unix/proc.c shell commands get executed
as:

        shell -c argv0 argv1 argv2

I believe it should be:

        shell -c "argv0 argv1 .."

I initially fixed the suexec problem this way ... because "shell -c
suexec user group ... " would never work (at least with my /bin/sh), 
but fixing it such that "shell -c 'suexec user group ... '" leads to 
simple exploits like :

<!--#exec cmd="somecmd ; evilcmd"-->

being trivial. I used the code in patch 4 (proc.c) to fix this for
me though (for the general non-suexec case) ... it might be desireable 
anyway , just to have exec cmd work in general. 

  4: http://redbrick.dcu.ie/~colmmacc/patches/proc.patch 

And finally , a whole bundle of patches related to the comment in the 
STATUS file:

* PR#1120: suexec
      suexec does not parse arguments to #exec cmd

I decided to make this work, for my own amusement. The result is rather
convoluted though , but if anyone is interested in resolving this issue, 
it's there. Basically just define a trusted system shell at buildtime
and have suexec allow it be used .. and have unixd.c detect shellcmd's
and warp what suexec gets sent on that basis. It's at:

  http://redbrick.dcu.ie/~colmmacc/patches/suexec-shell.patch

All of the patches are proving useful to us at least, but I would
say that a patch to mod_cgid should be a matter of priority for
the next release of apache, as it is currently a security hole.

-- 
colmmacc@redbrick.dcu.ie        PubKey: colmmacc+pgp@redbrick.dcu.ie  
Web:                                 http://devnull.redbrick.dcu.ie/ 

Re: suexec related patches for PR 7810, 7791, 8291, 9038

Posted by Brian Pane <bp...@pacbell.net>.
Thanks, I've committed patches 2 and 3.  I'll finish up the
others tomorrow unless someone else gets to them first.

--Brian

On Sat, 2002-05-25 at 20:17, Colm MacCárthaigh wrote:
> 
> Since there have been some changes to the affected source files
> and multiple problems presented themselves in unixd.c, my patches
> to make suexec + [ mod_include | mod_userdir | mod_cgid ] work
> were getting stale. So I've rediffed them against CVS.
> 
> I also had a good look through all of the suexec bugs, I'm using
> the patches on a production system now with over 2000 shell users
> (redbrick.dcu.ie) and it's proving stable.
> 
> Anyway, I think they fix these :
> 
>  PR 7810 - suexec + mod_userdir + mod_cgid needed fixing (also 
>            it's currently insecure by default, this really needs
>            to be fixed)
>  PR 7791 - malformed arguments array passed to suexec
>  PR 8291 - mod_include + suexec "exec cmd" not working
>  PR 9038 - really a duplicate of 7810
> 
> Some notes:
> 
>   1: http://redbrick.dcu.ie/~colmmacc/patches/mod_cgid.patch
>   2: http://redbrick.dcu.ie/~colmmacc/patches/unixd.patch
>   3: http://redbrick.dcu.ie/~colmmacc/patches/mod_include.patch
> 
>   patch 1 (mod_cgid.c)    fixes 7810/9039/mod_cgid, it just works.
>   patch 2 (unixd.c)       fixes 7791 and 8291 
>   patch 3 (mod_include.c) makes patch 2 secure. (otherwise include
>                           file="some.cgi" runs as the server user)
> Other Patches:
> 
> These are against 2.0.36, but should apply to CVS.   
> 
> Whilst trawling code for patch 2 I noticed that in 
> srclib/apr/threadproc/unix/proc.c shell commands get executed
> as:
> 
>         shell -c argv0 argv1 argv2
> 
> I believe it should be:
> 
>         shell -c "argv0 argv1 .."
> 
> I initially fixed the suexec problem this way ... because "shell -c
> suexec user group ... " would never work (at least with my /bin/sh), 
> but fixing it such that "shell -c 'suexec user group ... '" leads to 
> simple exploits like :
> 
> <!--#exec cmd="somecmd ; evilcmd"-->
> 
> being trivial. I used the code in patch 4 (proc.c) to fix this for
> me though (for the general non-suexec case) ... it might be desireable 
> anyway , just to have exec cmd work in general. 
> 
>   4: http://redbrick.dcu.ie/~colmmacc/patches/proc.patch 
> 
> And finally , a whole bundle of patches related to the comment in the 
> STATUS file:
> 
> * PR#1120: suexec
>       suexec does not parse arguments to #exec cmd
> 
> I decided to make this work, for my own amusement. The result is rather
> convoluted though , but if anyone is interested in resolving this issue, 
> it's there. Basically just define a trusted system shell at buildtime
> and have suexec allow it be used .. and have unixd.c detect shellcmd's
> and warp what suexec gets sent on that basis. It's at:
> 
>   http://redbrick.dcu.ie/~colmmacc/patches/suexec-shell.patch
> 
> All of the patches are proving useful to us at least, but I would
> say that a patch to mod_cgid should be a matter of priority for
> the next release of apache, as it is currently a security hole.
> 
> -- 
> colmmacc@redbrick.dcu.ie        PubKey: colmmacc+pgp@redbrick.dcu.ie  
> Web:                                 http://devnull.redbrick.dcu.ie/ 



Re: suexec, userdir, and mod_cgid Re: suexec related patches for PR 7810, 7791, 8291, 9038

Posted by Brian Pane <bp...@pacbell.net>.
On Sun, 2002-05-26 at 13:18, Brian Pane wrote:
> On Sat, 2002-05-25 at 20:17, Colm MacCárthaigh wrote:

> >   patch 1 (mod_cgid.c)    fixes 7810/9039/mod_cgid, it just works.
> 
> Patch 1 didn't work when I tried it.  The problem was that it depends
> on mod_userdir's get_suexec_id_doer() to create a proper suexec
> config based on the userdir.  But mod_userdir's get_suexec_id_doer()
> function never is invoked, because mod_suexec's get_suexec_id_doer()
> is called first.
> 
> I modified mod_userdir to insert its suexec identity function in
> front of mod_suexec's.  The suexec+cgid+userdir combination now works
> properly in my test environment, but I'd like a second opinion before
> I commit.

Actually, as Colm pointed out in PR 9083, the precedence of
mod_userdir vs mod_suexec is a more general design issue,
independent of mod_cgid.  With patch1 and without a mod_userdir
change, the combination of cgid+userdir+suexec works as long
as there's no global SuexecUserGroup directive.  That brings
cgid up to par with cgi, so I'll commit the cgid patch.

--Brian



suexec, userdir, and mod_cgid Re: suexec related patches for PR 7810, 7791, 8291, 9038

Posted by Brian Pane <bp...@pacbell.net>.
On Sat, 2002-05-25 at 20:17, Colm MacCárthaigh wrote:
> 
> Since there have been some changes to the affected source files
> and multiple problems presented themselves in unixd.c, my patches
> to make suexec + [ mod_include | mod_userdir | mod_cgid ] work
> were getting stale. So I've rediffed them against CVS.
> 
> I also had a good look through all of the suexec bugs, I'm using
> the patches on a production system now with over 2000 shell users
> (redbrick.dcu.ie) and it's proving stable.
> 
> Anyway, I think they fix these :
> 
>  PR 7810 - suexec + mod_userdir + mod_cgid needed fixing (also 
>            it's currently insecure by default, this really needs
>            to be fixed)
>  PR 7791 - malformed arguments array passed to suexec
>  PR 8291 - mod_include + suexec "exec cmd" not working
>  PR 9038 - really a duplicate of 7810
> 
> Some notes:
> 
>   1: http://redbrick.dcu.ie/~colmmacc/patches/mod_cgid.patch
>   2: http://redbrick.dcu.ie/~colmmacc/patches/unixd.patch
>   3: http://redbrick.dcu.ie/~colmmacc/patches/mod_include.patch
> 
>   patch 1 (mod_cgid.c)    fixes 7810/9039/mod_cgid, it just works.

Patch 1 didn't work when I tried it.  The problem was that it depends
on mod_userdir's get_suexec_id_doer() to create a proper suexec
config based on the userdir.  But mod_userdir's get_suexec_id_doer()
function never is invoked, because mod_suexec's get_suexec_id_doer()
is called first.

I modified mod_userdir to insert its suexec identity function in
front of mod_suexec's.  The suexec+cgid+userdir combination now works
properly in my test environment, but I'd like a second opinion before
I commit.

Thanks,
--Brian