+RTS options introduce a security problem for, e.g., setuid binaries

[andersk]

The fact that every ghc-compiled program accepts +RTS options could be a security problem in several contexts. For example, if you compile a “Hello, world!” program and make it setuid root, any user can now overwrite any file on the system using root privileges: hello +RTS -t/etc/passwd.

The GHCRTS environment variable has the same problem.

One should not need to have to know about these obscure features to write a secure program that accepts untrusted arguments.

[simonmar]

Incedentally, Python doesn't support setuid scripts, but it provides a wrapper program for doing this which removes any dodgy environment variables before invoking the interpreter:  http://svn.python.org/projects/python/trunk/Misc/setuid-prog.c. The same idea would be necessary for GHC.

I guess the problem for GHC is that you can easily get it wrong by accident, whereas setuid won't work for Python scripts by default.

[simonmar]

Ian, can this be closed now?

[igloo]

Yes, fixed by:

Tue Mar 16 23:33:57 GMT 2010  Ian Lynagh <igloo@earth.li>
  * Don't use -Bsymbolic when linking the RTS
  This makes the RTS hooks work when doing dynamic linking

Mon Mar 15 17:35:41 GMT 2010  Ian Lynagh <igloo@earth.li>
  * When saying RTS options are disabled, also say how to enable them

Sun Mar 14 13:36:48 GMT 2010  Ian Lynagh <igloo@earth.li>
  * Don't enable RTS options by default

Sat Mar 13 15:45:55 GMT 2010  Ian Lynagh <igloo@earth.li>
  * Add a link-time flag to en/disable the RTS options
  If RTS options are disabled then:
  * The ghc_rts_opts C code variable is processed as normal
  * The GHCRTS environment variable is ignored and, if it is defined, a
    warning is emitted
  * The +RTS flag gives an error and terminates the program

[nad]

This bug seems to have been closed prematurely. With GHC 7.0.1 RC2 (20101028):

$ echo "main = return ()" > Main.hs && ghc Main.hs && GHCRTS=-tfoo ./Main && cat foo

[1 of 1] Compiling Main             ( Main.hs, Main.o )
Linking Main ...
./Main +RTS -tfoo
<<ghc: 43556 bytes, 1 GCs, 26120/26120 avg/max bytes residency (1 samples), 1M in use, 0.00 INIT (0.00 elapsed), 0.00 MUT (0.00 elapsed), 0.00 GC (0.00 elapsed) :ghc>>

[igloo]

Ah, quite right, thanks.

I don't think GHCRTS is such a security issue, though, so this might be best left as part of #4243.

[simonmar]

I have a patch for this, coming soon.

[simonmar]

Fixed:

Thu Dec 16 11:44:52 GMT 2010  Simon Marlow <marlowsd@gmail.com>
  * fix #3910

[igloo]

Merged.

[guest]

I feel like the solution here was a bit overzealous. Now we have to compile things with -rtsopts just to turn on basic things like -N which should be reasonably safe.

Can we perhaps back this off to the point of requiring -rtsopts in order to set filepaths for profiling output, but allow other RTS options by default?

[andersk]

Replying to guest:

just to turn on basic things like -N which should be reasonably safe.

How do you know that passing -N to a setuid binary couldn’t be used to perform a denial of service attack by spawning many threads as root?

But in general, it should never be up to the compiler to say how the program compiled with it interprets its command-line arguments and environment variables. If the programmer wants to allow the user to change the program’s behavior in those ways, then the programmer needs to say so.

[simonmar]

I resent the fact that the broken CGI spec forced the pain of -rtsopts onto everyone, but we're stuck with it. Unfortunately most of the RTS options are "insecure" in some sense, whether it is by leaking information, DOS, or writing files.

Just alias ghc to ghc -rtsopts in your shell, that's what I do.

[duncan]

Patch: Use signed comparison for +RTS -N x <= 0 test

[duncan]

Patch: Change what +RTS options are available by default

[duncan]

I also find it annoying, and other users of parallel haskell find it annoying too. Attached is a patch to relax the situation somewhat.

Author: Duncan Coutts <duncan@well-typed.com>
Date:   Thu Oct 27 13:26:15 2011 +0100

Ticket #3910 originally pointed out that the RTS options are a potential
security problem. For example the -t -s or -S flags can be used to
overwrite files. This would be bad in the context of CGI scripts or
setuid binaries. So we introduced a system where +RTS processing is more
or less disabled unless you pass the -rtsopts flag at link time.
    
This scheme is safe enough but it also really annoies users. They have
to use -rtsopts in many circumstances: with -threaded to use -N, with
-eventlog to use -l, with -prof to use any of the profiling flags. Many
users just set -rtsopts globally or in project .cabal files. Apart from
annoying users it reduces security because it means that deployed
binaries will have all RTS options enabled rather than just profiling
ones.
    
This patch relaxes the set of RTS options that are available in the
default -rtsopts=some case. For "deployment" ways like vanilla and
-threaded we remain quite conservative. Only --info -? --help are
allowed for vanilla. For -threaded, -N and -N<x> are allowed with a
check that x <= num cpus.
    
For "developer" ways like -debug, -eventlog, -prof, we allow all the
options that are special to that way. Some of these allow writing files,
but the file written is not directly under the control of the attacker.
For the setuid case (where the attacker would have control over binary
name, current dir, local symlinks etc) we check if the process is
running setuid/setgid and refuse all RTS option processing. Users would
need to use -rtsopts=all in this case.
    
We are making the assumption that developers will not deploy binaries
built in the -debug, -eventlog, -prof ways. And even if they do, the
damage should be limited to DOS, information disclosure and writing
files like <progname>.eventlog, not arbitrary files.

[duncan@…]

commit 8c7ad0bd5bf1e7f62f44784cc889e8ee585b8d08

Author: Duncan Coutts <duncan@well-typed.com>
Date:   Thu Oct 27 13:26:15 2011 +0100

    Change what +RTS options are available by default
    
    Ticket #3910 originally pointed out that the RTS options are a potential
    security problem. For example the -t -s or -S flags can be used to
    overwrite files. This would be bad in the context of CGI scripts or
    setuid binaries. So we introduced a system where +RTS processing is more
    or less disabled unless you pass the -rtsopts flag at link time.
    
    This scheme is safe enough but it also really annoies users. They have
    to use -rtsopts in many circumstances: with -threaded to use -N, with
    -eventlog to use -l, with -prof to use any of the profiling flags. Many
    users just set -rtsopts globally or in project .cabal files. Apart from
    annoying users it reduces security because it means that deployed
    binaries will have all RTS options enabled rather than just profiling
    ones.
    
    This patch relaxes the set of RTS options that are available in the
    default -rtsopts=some case. For "deployment" ways like vanilla and
    -threaded we remain quite conservative. Only --info -? --help are
    allowed for vanilla. For -threaded, -N and -N<x> are allowed with a
    check that x <= num cpus.
    
    For "developer" ways like -debug, -eventlog, -prof, we allow all the
    options that are special to that way. Some of these allow writing files,
    but the file written is not directly under the control of the attacker.
    For the setuid case (where the attacker would have control over binary
    name, current dir, local symlinks etc) we check if the process is
    running setuid/setgid and refuse all RTS option processing. Users would
    need to use -rtsopts=all in this case.
    
    We are making the assumption that developers will not deploy binaries
    built in the -debug, -eventlog, -prof ways. And even if they do, the
    damage should be limited to DOS, information disclosure and writing
    files like <progname>.eventlog, not arbitrary files.

 rts/RtsFlags.c |  131 +++++++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 110 insertions(+), 21 deletions(-)

[duncan]

Replying to andersk:

Replying to guest:

just to turn on basic things like -N which should be reasonably safe.

How do you know that passing -N to a setuid binary couldn’t be used to perform a denial of service attack by spawning many threads as root?

We've limited it to the number of CPUs in the box. That could still be a denial of service but now at least somewhat limited.

Note also that for setuid binaries we now disallow all +RTS options in the default -rtsopts=some mode.

Hopefully this gives us a reasonable balance between convenience and security. Opinions on refinements welcome.