search

aeppert / gyp

Automatically exported from code.google.com/p/gyp
0 stars 0 forks source link

ninja generator uses insecure rpath by default #315

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
gyp uses insecure rpaths by default for Ninja projects. This blocks Ninja for 
being used by Chrome OS builds.

Reproduction recipe: Build Chrome with ninja and observe the command-line for 
chrome-sandbox:

 $ ninja -C c/Release chrome_sandbox -v
[1/1] flock linker.lock x86_64-cros-linux-gnu-g++ -Wl,-z,now -Wl,-z,relro 
-pthread -Wl,-z,noexecstack -fPIC -B../../third_party/gold 
--sysroot=/build/lumpy/ -Wl,-O1 -Wl,--as-needed -Wl,--gc-sections -o 
chrome_sandbox -Wl,-rpath=\$ORIGIN/lib -Wl,--start-group 
obj/sandbox/linux/suid/chrome_sandbox.linux_util.o 
obj/sandbox/linux/suid/chrome_sandbox.process_util_linux.o 
obj/sandbox/linux/suid/chrome_sandbox.sandbox.o  -Wl,--end-group 

You can see from the above commandline that an rpath of $ORIGIN/lib is used, 
which is insecure. This presents security problems since chrome-sandbox is a 
setuid executable. If you generate the same executable with make, it does not 
have this insecure rpath.

You can see the bug here: src/tools/gyp/pylib/gyp/generator/ninja.py
    master_ninja.rule(
      'link',
      description='LINK $out',
      command=('$ld $ldflags -o $out -Wl,-rpath=\$$ORIGIN/lib '
               '-Wl,--start-group $in $solibs -Wl,--end-group $libs'))

This results in an insecure rpath. For comparison, the gyp generator for make 
does not add the same rpath into chrome-sandbox.

The reason why these rpaths are insecure is because hardlinks preserve setuid 
bits, so an unprivileged user could gain root privileges by hardlinking a 
setuid executable and then adding in whatever binaries they want to run into 
the lib directory. Example bug report: 
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=520126

Original issue reported on code.google.com by davidjames@chromium.org on 15 Dec 2012 at 1:22

GoogleCodeExporter commented 9 years ago 
From the make generator:

          if any(dep.endswith('.so') for dep in deps):
            # We want to get the literal string "$ORIGIN" into the link command,
            # so we need lots of escaping.
            ldflags.append(r'-Wl,-rpath=\$$ORIGIN/lib.%s/' % self.toolset)
            ldflags.append(r'-Wl,-rpath-link=\$(builddir)/lib.%s/' %
                           self.toolset)

Are you sure the make generator isn't doing this?

Original comment by thakis@chromium.org on 15 Dec 2012 at 5:09

GoogleCodeExporter commented 9 years ago 
The make generator only adds the insecure rpath when it is needed. This makes 
it possible to generate a secure setuid executable using make.

Using ninja, the insecure rpath is on by default and can't be turned off -- 
that's really the bug here.

Original comment by davidjames@chromium.org on 10 Jan 2013 at 5:20

GoogleCodeExporter commented 9 years ago 
Is this still an issue? (ChromeOS now uses ninja.)

Original comment by thakis@chromium.org on 30 Apr 2014 at 5:35

GoogleCodeExporter commented 9 years ago 
yes, this is still a problem.  the ninja generator does:
http://code.google.com/p/gyp/source/browse/trunk/pylib/gyp/generator/ninja.py?r=
1895#1075

      if is_executable and len(solibs):
        ...
        ldflags.append('-Wl,-rpath=\$$ORIGIN/%s' % rpath)

which is no good

Original comment by vapier@chromium.org on 2 Jun 2014 at 8:14

GoogleCodeExporter commented 9 years ago 
The len(solibs) is like the any(dep.endswith('.so')) in make though, right?

…actually, now that I look at where that line is from, cwolfe did fix this 
1.5 years ago as far as I can tell: https://codereview.chromium.org/11821060/ 
(mentions this bug in the BUG line)

Please shout if you think this is still an issue!

Original comment by thakis@chromium.org on 17 Jun 2014 at 11:00

GoogleCodeExporter commented 9 years ago 
Thanks for fixing this. That's exactly what we were looking for.

Original comment by davidjames@chromium.org on 17 Jun 2014 at 11:02

GoogleCodeExporter commented 9 years ago 
i don't see how this is fixed.  that the make targets also use insecure rpaths 
just means the make targets are also bad.  that ninja now behaves the same as 
make means they both use insecure rpaths.

we don't want -rpath $ORIGIN at all.  gyp provides no way of supporting this.

Original comment by vapier@chromium.org on 18 Jun 2014 at 12:21

GoogleCodeExporter commented 9 years ago 
It does: don't depend on shared libraries.

Comment 0 says: "If you generate the same executable with make, it does not 
have this insecure rpath."

So this big sounds fixed. If you need more control over rpath, that sounds like 
a separate bug.

Original comment by thakis@chromium.org on 18 Jun 2014 at 12:28

GoogleCodeExporter commented 9 years ago 
the title of the bug is "ninja generator uses insecure rpath by default" and it 
still does exactly that.  saying "don't use shared libs" is pretty ridiculous.

if you want to just fork the problem into a diff bug, then feel free to do 
that.  gyp is still producing insecure binaries.

Original comment by vapier@chromium.org on 18 Jun 2014 at 1:02

GoogleCodeExporter commented 9 years ago 
For this particular bug, my use case (for Chrome OS) was that we wanted to 
generate a static setuid executable that did not use rpath. This is needed for 
Chrome OS to be secure when using ninja. We are now able to do this so the use 
case is met.

I agree that it's counter-intuitive that adding a .so file to chrome-sandbox 
would suddenly make it insecure. It's up to you guys how you want to manage the 
bugs, but the immediate and urgent need is now met.

Original comment by davidjames@chromium.org on 18 Jun 2014 at 1:18