From d77c0392efd949057ff68b31e418a578fb9c3ac3 Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Sun, 4 Aug 2024 05:54:50 +0800 Subject: [PATCH] linkage_checker: replace `Fiddle.dlopen` with `libSystem` call `dlopen`ing a library executes potentially untrusted code (e.g. if the library has initialisers). We can avoid the `dlopen` call by asking `libSystem` directly about whether a library can be found in the shared cache. Of course, the `dlopen` happens after a `ENOENT`, so the attack surface here is relatively small. But relying on this still exposes us to a potential TOCTOU[^1] bug. Let's avoid it entirely by skipping `dlopen` altogether. Also: add RBI for `Fiddle` constants used in `linkage_checker` Upstream don't have these definitions yet, so I've added an RBI for them in the meantime. [^1]: https://en.wikipedia.org/wiki/Time-of-check_to_time-of-use --- Library/Homebrew/linkage_checker.rb | 21 ++++++++++++++------- Library/Homebrew/sorbet/rbi/upstream.rbi | 14 ++++++++++++++ 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/Library/Homebrew/linkage_checker.rb b/Library/Homebrew/linkage_checker.rb index 7d986104247e2..6d11765b2b99f 100644 --- a/Library/Homebrew/linkage_checker.rb +++ b/Library/Homebrew/linkage_checker.rb @@ -160,9 +160,9 @@ def check_dylibs(rebuild_cache:) if (dep = dylib_to_dep(dylib)) @broken_deps[dep] |= [dylib] - elsif system_libraries_exist_in_cache? && dylib_found_via_dlopen(dylib) + elsif system_libraries_exist_in_cache? && dylib_found_in_shared_cache?(dylib) # If we cannot associate the dylib with a dependency, then it may be a system library. - # If dlopen finds the dylib, then the linkage is not broken. + # Check the dylib shared cache for the library to verify this. @system_dylibs << dylib elsif !system_framework?(dylib) && !broken_dylibs_allowed?(file.to_s) @broken_dylibs << dylib @@ -195,11 +195,18 @@ def system_libraries_exist_in_cache? end alias generic_system_libraries_exist_in_cache? system_libraries_exist_in_cache? - def dylib_found_via_dlopen(dylib) - Fiddle.dlopen(dylib).close - true - rescue Fiddle::DLError - false + def dylib_found_in_shared_cache?(dylib) + @dyld_shared_cache_contains_path ||= begin + libc = Fiddle.dlopen("/usr/lib/libSystem.B.dylib") + + Fiddle::Function.new( + libc["_dyld_shared_cache_contains_path"], + [Fiddle::TYPE_CONST_STRING], + Fiddle::TYPE_BOOL, + ) + end + + @dyld_shared_cache_contains_path.call(dylib) end def check_formula_deps diff --git a/Library/Homebrew/sorbet/rbi/upstream.rbi b/Library/Homebrew/sorbet/rbi/upstream.rbi index a00487f95aced..b35381d59ca2b 100644 --- a/Library/Homebrew/sorbet/rbi/upstream.rbi +++ b/Library/Homebrew/sorbet/rbi/upstream.rbi @@ -2,3 +2,17 @@ # This file contains temporary definitions for fixes that have # been submitted upstream to https://github.com/sorbet/sorbet. + +# Missing constants we use in `linkage_checker.rb` +# https://github.com/sorbet/sorbet/pull/8215 +module Fiddle + # [`TYPE_BOOL`](https://github.com/ruby/fiddle/blob/v1.1.2/ext/fiddle/fiddle.h#L129) + # + # C type - bool + TYPE_BOOL = T.let(T.unsafe(nil).freeze, Integer) + + # [`TYPE_CONST_STRING`](https://github.com/ruby/fiddle/blob/v1.1.2/ext/fiddle/fiddle.h#L128) + # + # C type - char\* + TYPE_CONST_STRING = T.let(T.unsafe(nil).freeze, Integer) +end