Skip to content
Permalink
Browse files Browse the repository at this point in the history
fix: Do not run sensible-pager as root if using sudo/pkexec
The apport-cli supports view a crash. These features invoke the default
pager, which is likely to be less, other functions may apply.

It can be used to break out from restricted environments by spawning an
interactive system shell. If the binary is allowed to run as superuser
by sudo, it does not drop the elevated privileges and may be used to
access the file system, escalate or maintain privileged access.

apport-cli should normally not be called with sudo or pkexec. In case it
is called via sudo or pkexec execute `sensible-pager` as the original
user to avoid privilege elevation.

Proof of concept:

```
$ sudo apport-cli -c /var/crash/xxx.crash
[...]
Please choose (S/E/V/K/I/C): v
!id
uid=0(root) gid=0(root) groups=0(root)
!done  (press RETURN)
```

This fixes CVE-2023-1326.

Bug: https://launchpad.net/bugs/2016023
Signed-off-by: Benjamin Drung <benjamin.drung@canonical.com>
  • Loading branch information
bdrung committed Apr 12, 2023
1 parent ee3e896 commit e5f78cc
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 8 deletions.
15 changes: 11 additions & 4 deletions apport/ui.py
Expand Up @@ -119,15 +119,19 @@ def _get_users_environ(uid: int) -> dict[str, str]:
}


def run_as_real_user(args: list[str]) -> None:
def run_as_real_user(
args: list[str], *, get_user_env: bool = False, **kwargs
) -> None:
"""Call subprocess.run as real user if called via sudo/pkexec.
If we are called through pkexec/sudo, determine the real user ID and
run the command with it to get the user's web browser settings.
If get_user_env is set to True, the D-BUS address and XDG_DATA_DIRS
is grabbed from a running gvfsd and added to the process environment.
"""
uid = _get_env_int("SUDO_UID", _get_env_int("PKEXEC_UID"))
if uid is None or not get_process_user_and_group().is_root():
subprocess.run(args, check=False)
subprocess.run(args, check=False, **kwargs)
return

pwuid = pwd.getpwuid(uid)
Expand All @@ -140,7 +144,9 @@ def run_as_real_user(args: list[str]) -> None:
k: v
for k, v in os.environ.items()
if not k.startswith("SUDO_") and k != "PKEXEC_UID"
} | _get_users_environ(uid)
}
if get_user_env:
env |= _get_users_environ(uid)
env["HOME"] = pwuid.pw_dir
subprocess.run(
args,
Expand All @@ -149,6 +155,7 @@ def run_as_real_user(args: list[str]) -> None:
user=uid,
group=gid,
extra_groups=os.getgrouplist(pwuid.pw_name, gid),
**kwargs,
)


Expand Down Expand Up @@ -1817,7 +1824,7 @@ def open_url(self, url):

try:
try:
run_as_real_user(["xdg-open", url])
run_as_real_user(["xdg-open", url], get_user_env=True)
except OSError:
# fall back to webbrowser
webbrowser.open(url, new=True, autoraise=True)
Expand Down
3 changes: 1 addition & 2 deletions bin/apport-cli
Expand Up @@ -185,9 +185,8 @@ class CLIUserInterface(apport.ui.UserInterface):
self.in_update_view = True
report = self._get_details()
try:
subprocess.run(
apport.ui.run_as_real_user(
["/usr/bin/sensible-pager"],
check=False,
input=report.encode("UTF-8"),
stdout=stdout,
)
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/test_ui.py
Expand Up @@ -2717,7 +2717,7 @@ def test_run_as_real_user(self) -> None:
with unittest.mock.patch(
"subprocess.run", side_effect=mock_run_calls_except_pgrep
) as run_mock:
run_as_real_user(["/bin/true"])
run_as_real_user(["/bin/true"], get_user_env=True)

run_mock.assert_called_with(
["/bin/true"],
Expand Down Expand Up @@ -2755,7 +2755,7 @@ def test_run_as_real_user_no_gvfsd(
with unittest.mock.patch(
"subprocess.run", side_effect=mock_run_calls_except_pgrep
) as run_mock:
run_as_real_user(["/bin/true"])
run_as_real_user(["/bin/true"], get_user_env=True)

run_mock.assert_called_with(
["/bin/true"],
Expand Down

0 comments on commit e5f78cc

Please sign in to comment.