mesonbuild / meson

The Meson Build System
http://mesonbuild.com
Apache License 2.0
5.63k stars 1.63k forks source link

LC_RPATH for external library is stripped from the installed binary #12045

Closed jmillan closed 7 months ago

jmillan commented 1 year ago

Describe the bug When using -Db_sanitize=address the binary created in the build directory contains the LC_RPATH definitions added by clang due to the -fsanitize option provided, which in my case is /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/14.0.3/lib/darwin/ , but the binary in the install directory does not.

To Reproduce meson.build:

project('tutorial', 'c')
executable('demo','main.c',
  build_by_default: true,
  install: true,
  install_tag: 'demo')

main.c

#include <stdio.h>

int main(int argc, char **argv) {
  printf("Hello there.\n");
  return 0;
}
  1. meson setup --prefix ${PWD}/bin -Db_sanitize=address -Db_lundef=false build
  2. meson compile -C build/
  3. meson install -C build
  4. Verify LC_RPATH in the binary located in build directory (otool -l build/demo | grep -A3 PATH). It contains the definitions of LC_RPATH.
  5. Verify LC_RPATH in the binary located in install directory (otool -l bin/bin/demo | grep -A3 PATH). It does not contain the definitions of LC_RPATH.

Expected behavior LC_RPATH should be also defined in the binary located in the installation path.

system parameters

tzahola commented 1 year ago

I think the issue is that in fix_darwin Meson removes all LC_RPATHs from the binary, but then only adds back those rpaths that are coming from Meson (i.e. install_name_mappings). In my opinion Meson shouldn't remove those rpaths that aren't coming from Meson targets, in which case -fsanitize's implicitly-added rpath would be left there.

grobian commented 8 months ago

I see the same problem for packages using meson here too using Gentoo Prefix on macOS.

In Gentoo Prefix we use a linker wrapper that always appends a bunch of rpath arguments to make sure libaries from the "Prefix" are always found, but meson removes them.

Eli Schwartz @eli-schwartz has taken the time to explain this behaviour quite detailed in https://bugs.gentoo.org/923706:

Well, it has to remove unsafe rpaths, i.e. any rpath that was generated on the build machine in order to make uninstalled / test usage work. Consider, for example, distros where packages are built in ~/packages/foobar/ and extracted to ~/packages/PN/workdir/PN-PV/

When building the package, rpaths are generated for /home/jessica/packages/foobar/workdir/foobar-1.0

The result is built into a binary package and distributed via the package manager channels to all users of $DISTRO, which means that all users of $DISTRO can install a program that will attempt to read its libraries from the "jessica" user's home directory. This is a CVE: on shared user systems, one simply requests an account with the username "jessica" and gains the ability to trick other users into running arbitrary attack code.

What about on Gentoo, if a binhost builds packages with a PORTAGE_TMPDIR that is different from the one on end user machines? My /var/tmp/portage is not writable by all users, but /var/tmp is -- and if I changed it to /var/lib/portage/tmp then does that mean that any user can now create files in /var/tmp/portage that clash with the rpaths produced by a binhost?

So, rpaths that are only meant to be used for uninstalled / test usage at build time do in fact need to be removed at install time. The question is then not whether rpaths should be removed, but *which* rpaths should be removed.

So my understanding here is that meson is "security minded", but this mindedness interferes with common practices to linking, where the toolchain via one way or another adds references to the linker here.

What I could see happening is: a) meson to remember what it added to make the objects work during build/test, and remove those paths again, this does not cater for the security aspect, but makes meson "mind its own business". b) meson to simply re-link on install, this way it avoids any trickery with tools to modify objects, and instead lets the linker re-generate an object that this time only has the rpaths that are meant to be there. This can then comply to the security minding of meson because it can check no weird paths (like /Users/jessica) are added, while it allows the toolchain to append whatever paths are necessary to function in the environment being built for.

I guess the crux in these kinds of scenarios is that meson will not interfere with the toolchain's doing, because the toolchain does things that must be done, that meson cannot (or should not have to) have knowledge about.

grobian commented 8 months ago

there is another way I can see:

c) meson can compare rpaths in each object after linking and the rpaths it passed onto the linker, and add the surplus of rpaths to its internal state to retain on cleanup.

It should do this not exclusively on macOS, also ELF platforms can have this problem.

tzahola commented 8 months ago

B is the correct answer. Meson shouldn't try post-hoc fixing the uninstalled/test binary by removing rpaths at install time. It should relink the binary from scratch without passing the meson-related rpaths to the linker.

If you want to make scrambled eggs, then make scrambled eggs. If you want to make boiled eggs, then boil them. But you shouldn't try to reassemble scrambled eggs to boil them instead.

grobian commented 7 months ago

Since ELF has basically approach A, above a patch that implements A for Mach-O.

I still feel relinking is safer, but that would require a much larger change I suppose than this which should be in line with main ELF behaviour.

eli-schwartz commented 7 months ago

So my understanding is that this problem was long ago fixed for ELF, but that fix never carried over to darwin, and the linked PR implements that for darwin too.

That is a nasty failure mode. Thanks for debugging it, @grobian, your fix looks absolutely correct to me.