Skip to content
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

Loader path security #1012

Merged
merged 17 commits into from
Dec 15, 2023
Merged

Loader path security #1012

merged 17 commits into from
Dec 15, 2023

Conversation

mrdomino
Copy link
Collaborator

@mrdomino mrdomino commented Dec 15, 2023

The ape loader now passes the program executable name directly as a register. x2 is used on aarch64, %rdx on x86_64. This is passed as the third argument to cosmo() (M1) or Launch (non-M1) and is assigned to the global __program_executable_name.

GetProgramExecutableName now returns this global's value, setting it if it is initially null. InitProgramExecutableName first tries exotic, secure methods: KERN_PROC_PATHNAME on FreeBSD/NetBSD, and /proc on Linux. If those produce a reasonable response (i.e., not "/usr/bin/ape", which happens with the loader before this change), that is used. Otherwise, if issetugid(), the empty string is used. Otherwise, the old argv/envp parsing code is run.

The value returned from the loader is always the full absolute path of the binary to be executed, having passed through realpath. For the non-M1 loader, this necessitated writing RealPath, which uses readlinkat of "/proc/self/fd/[progfd]" on Linux, F_GETPATH on Xnu, and the __realpath syscall on OpenBSD. On FreeBSD/NetBSD, it punts to GetProgramExecutableName, which is secure on those OSes.

With the loader, all platforms now have a secure program executable name. With no loader or an old loader, everything still works as it did, but setuid/setgid is not supported if the insecure pathfinding code would have been needed.

Fixes #991.

The M1 loader now passes the program executable name directly as x2,
which is the third argument to `cosmo()`. From there it is assigned
to `__myname`.

`GetProgramExecutableName` now always returns `__myname`, and it is
`InitProgramExecutableName`’s job to set that variable if it is not
already assigned.

`InitProgramExecutableName` now dies if `issetugid()` and `__myname`
is null, pending a rework of the path-finding logic.
@mrdomino mrdomino changed the title M1 register M1 program name register Dec 15, 2023
It prints its own name to stderr, not stdout.

It also prints `GetProgramExecutableName()` for further debugging
convenience.
@mrdomino mrdomino changed the title M1 program name register M1 loader passes program name via register Dec 15, 2023
@mrdomino mrdomino mentioned this pull request Dec 15, 2023
- Tries `KERN_PROC_PATHNAME` first on NetBSD/FreeBSD.

- Tries procfs first, but now only on Linux. (You can have /proc on
  FreeBSD, but then again, /proc could be anything on non-Linux. An
  option here is to put non-Linux /proc after the issetugid check.)

- If `issetugid()` and none of the secure methods have worked, sets
  the empty string and returns.
Copy link
Owner

@jart jart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks!

@mrdomino mrdomino changed the title M1 loader passes program name via register Loader path security Dec 15, 2023
@mrdomino mrdomino force-pushed the m1-register branch 2 times, most recently from 5246a1b to 173c9aa Compare December 15, 2023 16:33
Using `COSMOPOLITAN_PROGRAM_EXECUTABLE` without a `sys_faccessat` was
rightly causing test failures. Now that it is no longer used by the
loader, there seems little value in prioritizing it over argv[0], so
just lump it in with "_".
@mrdomino mrdomino merged commit f94c11d into jart:master Dec 15, 2023
5 checks passed
@mrdomino mrdomino deleted the m1-register branch December 15, 2023 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Loader path security
2 participants