Unset the DYLD_LIBRARY_PATH env var when running Sorbet#820
Conversation
There are cases where a dev environment can set the `DYLD_LIBRARY_PATH` env variable in a way that loads a version of the C++ standard library that is not compatible with the system version of the library used by Sorbet. This will cause Sorbet to generate corrupted JSON if the --print=symbol-table-json flag is passed. Unsetting this environment variable forces Sorbet to default to the system version of the C++ standard library, preventing this bug.
| # standard library other than the one used by Sorbet, it will cause Sorbet to generate | ||
| # corrupted JSON. Unsetting this environment variable forces Sorbet to default to the system | ||
| # version of the C++ standard library. | ||
| unset_dyld_library_path = "env -u DYLD_LIBRARY_PATH" |
There was a problem hiding this comment.
Another question -- would it be worth unsetting more than just this env var? I don't want to risk unsetting something that Sorbet actually needs, but at the same time, it seems like this might not be the only env var that could load harmful C++ dependencies by accident?
There was a problem hiding this comment.
I wouldn't do it this way, since it isn't portable. popen3 and friends use Process.spawn in the backend, so they all have the ability to set new env variables for the duration of the spawn, which is what I would use inside exec and bundle_exec.
Moreover, I doubt running sorbet needs any env variables, so I would lean towards using unsetenv_others option in the popen3 implementation to unset all env vars.
|
Could someone legitimately want to set the DYLD path if they only have a custom installation of the C++ stdlib in their system? And if not, should we erase the variable with |
|
@vinistock I think you're right -- they might also have other things in their |
|
I think this was the right approach, tbh. I think we can safely remove all env variables from the Sorbet invocation. |
|
Yeah, for clarity, I was raising these questions not as a way to point anything wrong with the approach. I truly don't know if someone might want to set these variables manually for a custom C++ installation. However, I don't think we can remove more environment variables, especially anything related to the |
3 participants
There are cases where a dev environment can set the
DYLD_LIBRARY_PATHenv variable in a way that loads a version of the C++ standard library that is not compatible with the system version of the library used by Sorbet. This will cause Sorbet to generate corrupted JSON if the --print=symbol-table-json flag is passed. Unsetting this environment variable forces Sorbet to default to the system version of the C++ standard library, preventing this bug.Major question: how to test this?
I tried testing this by setting an env var on a
Contextobject, and then seeing if it would be unset in the context of aContext.srbcall, but I believe that every invocation ofContext.execcreates a new shell env that doesn't persist between calls (correct me if I'm wrong here), so none of my attempts were successful. Would appreciate thoughts here!