Run brew commands as regular user

[Neved4]

Fixes #382, related #1273.

Instead of running Homebrew as the root user, this change runs it as a normal user, which is the intended use case for Homebrew. After the change, the command is run a non-root user context, which seems a way to effectively drop privileges.

This prevents raising the error:

Error: Running Homebrew as root is extremely dangerous and no longer supported.
As Homebrew does not drop privileges on installation you would be giving all
build scripts full access to your system.

Note: the function could have a different descriptive name, refactored to be more compact, etc. See:

RunBrewAsUser() {
    { [ "$(id -u)" -eq 0 ] && sudo -u "$SUDO_USER" brew "$@"; } || brew "$@"
}

---

Security

Thanks to brew's safeguards this might not be exploitable, but including the following for reference.

MITRE's CWE-250 is related to this change:

CWE ID	CWE Title	Description
CWE-250	Execution with Unnecessary Privileges	The software runs with more privileges than necessary, which can lead to security vulnerabilities.

This change follows REF-76 by dropping the privileges as soon as possible, as in:

Run your code using the lowest privileges that are required to accomplish the necessary tasks.
Wrap and centralize this functionality if possible, and isolate the privileged code as much as possible from other code.
Raise privileges as late as possible, and drop them as soon as possible to avoid CWE-271.

Source: CWE-250: Execution with Unnecessary Privileges.

[wileyhy]

So this function: RunBrewAsUser() { { "$(id -u)" && sudo -u "$SUDO_USER" brew "$@"; } || brew "$@" } ...begins by expanding the command substitution, `"$(id -u)"`. In Bash 5.2 in Posix mode, this comsub outputs '10151' to Stdout. On a Linux machine it might print something in the low 1000's range. Any code before '&&' is read by the shell as one entire command, so, since this particular command line contains just one string, the shell will most likely search PATH for a command named 10151 (unless there is an alias or function by that name). This is probably not what was intended. In Linux, system commands can begin with digits, but neither 10151 nor 1000 nor 0 are common command names, so a bad actor could place a malicious script or program in PATH which the kernel would then attempt to execute. If lynis had been executed with root permissions, and if such a malicious script or program as this existed, then, my understanding at this time is that the kernel would be instructed by the user to execute such a program with root privileges. ? Wiley

…

On Mon, Oct 21, 2024, 05:40 ⑆ Neveda ⑈ ***@***.***> wrote: Runs Homebrew commands as original user when Lynis is invoked as root Fixes #382 <#382>, related #1273 <#1273>. Instead of running Homebrew as the root user, this change runs it as a normal user, which is the intended use case for Homebrew. After the change, the command is run a non-root user context, which seems a way to effectively drop privileges. Note The function could have a different or more descriptive name, refactored to be more compact, etc. See: RunBrewAsUser() { { "$(id -u)" && sudo -u "$SUDO_USER" brew "$@"; } || brew "$@" } ------------------------------ Security Thanks to brew's safeguards this might not be exploitable, but including the following information for reference. MITRE CWE-250 <https://cwe.mitre.org/data/definitions/250.html> is related to this change: CWE ID CWE Title Description CWE-250 <https://cwe.mitre.org/data/definitions/250.html> Execution with Unnecessary Privileges The software runs with more privileges than necessary, which can lead to security vulnerabilities. This change follows REF-76 <https://web.archive.org/web/20211209014121/https://www.cisa.gov/uscert/bsi/articles/knowledge/principles/least-privilege> by dropping the privileges as soon as possible: *Run your code using the lowest privileges that are required to accomplish the necessary tasks.* *Wrap and centralize this functionality if possible, and isolate the privileged code as much as possible from other code.* *Raise privileges as late as possible, and drop them as soon as possible to avoid CWE-271 <https://cwe.mitre.org/data/definitions/271.html>.* Source: *CWE-250: Execution with Unnecessary Privileges* <https://cwe.mitre.org/data/definitions/250.html>. ------------------------------ You can view, comment on, or merge this pull request online at: #1564 Commit Summary - ec13cf8 <ec13cf8> Run brew as regular user in place of root File Changes (2 files <https://github.com/CISOfy/lynis/pull/1564/files>) - *M* include/functions <https://github.com/CISOfy/lynis/pull/1564/files#diff-6b1c9d5fa25dcef458956914b06d9a272a7dbb697dde7953818b368df3068168> (17) - *M* include/tests_ports_packages <https://github.com/CISOfy/lynis/pull/1564/files#diff-ac2f15c8a8f71ea9668c7b5bc1640e1273fd5c288e7925974d482e3f48ca13a1> (4) Patch Links: - https://github.com/CISOfy/lynis/pull/1564.patch - https://github.com/CISOfy/lynis/pull/1564.diff — Reply to this email directly, view it on GitHub <#1564>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AUF2F27MGYTAIFZ5TIJAZWDZ4TY5LAVCNFSM6AAAAABQKCQZHKVHI2DSMVQWIX3LMV43ASLTON2WKOZSGYYDENBSHE3DCNA> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[wileyhy]

Have I gotten anything wrong in this reply? It's been kind of a weird day.

[Neved4]

Hey @wileyhy thx for looking into this! You're 100% right for noting it.

Additionally:

1. The function above is not the one that is in the commit.
2. You're right $(id -u) evaluates as-is instead of being compared to anything there.
3. This is a typo. It should have had the [ "$(id -u)" -eq 0 ] comparison.

We could edit my post to update this suggestion, or remove it since it doesn't reflect the current change.

Cheers!

[wileyhy]

Hi @Neveda, If Homebrew is installed on a system, then perhaps util-linux could also be installed from \brew\? That might allow for dropping privileges by use of \runuser\, \setpriv\ or \su\, assuming Homebrew's version has any of those binaries. I don't know for certain at the moment, but dropping privileges in a strict POSIX _shell_ context might not be possible. C17 is specified by POSIX, but how else could it be done? Wiley

…

On Tue, Oct 22, 2024, 02:36 ⑆ Neveda ⑈ ***@***.***> wrote: Hey @wileyhy <https://github.com/wileyhy> thx for looking into this! You're 100% right for noting it. Additionally: 1. The function above is not the one that is in the commit. 2. You're right $(id -u) evaluates as-is instead of being compared to anything there. 3. This is a typo. It should have had the [ "$(id -u)" -eq 0 ] comparison. We could edit my post to update this suggestion, or remove it since it doesn't reflect the current change. Cheers! — Reply to this email directly, view it on GitHub <#1564 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AUF2F26HAJSTIT7XVAD73FTZ4YMC3AVCNFSM6AAAAABQKCQZHKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMRYG44DGMZXGE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[Neved4]

Hi @wileyhy,

You're right to raise those points. My initial PR only adressed cases where lynis runs with sudo. We could consider POSIX more broadly.

In Homebrew, some packages like util-linux vary between macOS and Linux. You can find disabled binaries (util-linux.rb#L104-L132) in the util-linux Homebrew version, which also disables commands such as su and runuser on both systems (util-linux.rb#L88-L89). Interestingly, setpriv is not installed on macOS, and for Linux, it seems it doesn't override existing system commands.

In systems with only su available, we could do:

su "$(id -un)" -c 'id -u'

These are the shells it seems to be working in:

Shell	Version	Supported
bash	5.2.37	Yes
dash	0.5.12	Yes
etsh	5.4.0	No
ksh93	1.0.10	Yes
mksh	59c	Yes
oksh	7.5	Yes
osh	0.23.0	Yes
posh	0.14.1	Yes
yash	2.57	Yes
zsh	5.9	Yes

Both this one and sudo -u "$(whoami)" <cmd> seem to be effective ways to drop privileges temporarily for a specific command. On the flip side, neither sudo nor su are specified by POSIX.

C17 is specified by POSIX, but how else could it be done?

That's a great question. In C, I believe one can drop privileges by doing:

#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>

int main(int argc, char *argv[], char *environ[]) {
    uid_t prev_uid = getuid();
    uid_t prev_gid = getgid();

    if (setgid(prev_gid) != 0 || setuid(prev_uid) != 0) {
        perror("error: failed to drop privileges");
        return 1;
    }

    char *args[] = { "id", "-u", NULL };

    if (execve("/usr/bin/id", args, environ) == -1) {
        perror("error: failed to run execve");
        return 1;
    }

    return 0;
}

*This is test code and should not be used in production

Which seems to return the user id, even when calling it with sudo ./main. It also doesn't throw an error when running brew list.

Probably C folks would know what the right way to go about this is.

Currently, Homebrew only runs on both Linux and macOS. It's fair to assume brew is not running in a system without sudo, or that lynis would not call brew in a system that is not macOS nor Linux.

Using which isn’t POSIX-compliant, maybe we should use command -v instead. Besides that, the brew query in lynis only checks if the command itself is available, as in test_ports_packages#L1119:

lynis/include/tests_ports_packages

Line 119 in 3c9b379

FIND=$(which brew 2> /dev/null | grep -v "no [^ ]* in ")

It doesn’t check to skip the step for non-Linux or non-macOS systems.

Would be more than happy to add platform checks, use su instead of sudo or take any other approach. Feel free to make any adjustments necessary, or let me know what direction you’d prefer.

I’m here if you need anything.

Kind regards,
Neved4