-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
linkage_checker: replace Fiddle.dlopen
with libSystem
call
#18486
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Been meaning to look into this for a while.
Added an RBI patterned after upstream's to handle with the |
9aa5041
to
54fec35
Compare
Usually we add it to Seems like it's missing quite a few. |
Their definitions seemed tied to Ruby 2.7, so wasn't sure if sending it upstream was the right move. Will go ahead and do that. |
It was just initially imported from Ruby 2.7. Adding new constants from 3.3 is fine - I've done so before. |
54fec35
to
6acd7ec
Compare
Done in sorbet/sorbet#8215. |
`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
6acd7ec
to
d77c039
Compare
brew style
with your changes locally?brew typecheck
with your changes locally?brew tests
with your changes locally?dlopen
ing a library executes potentially untrusted code (e.g. if thelibrary has initialisers). We can avoid the
dlopen
call by askinglibSystem
directly about whether a library can be found in the sharedcache.
Of course, the
dlopen
happens after aENOENT
, so the attack surface hereis relatively small. But relying on this still exposes us to a potential
TOCTOU1 bug. Let's avoid it entirely by skipping
dlopen
altogether.brew typecheck
currently returns an error. Looks like the RBI for Fiddle will need updating.Footnotes
https://en.wikipedia.org/wiki/Time-of-check_to_time-of-use ↩