sudo-1.9.5p1

[Douglas R. Reno]

New point version

[Douglas R. Reno]

Now 1.9.5p1

[Douglas R. Reno]

Major changes between version 1.9.5p1 and 1.9.5:

    Fixed a regression introduced in sudo 1.9.5 where the editor run by sudoedit was set-user-ID root unless SELinux RBAC was in use. The editor is now run with the user's real and effective user-IDs. 

Major changes between version 1.9.5 and 1.9.4p2:

    Fixed a crash introduced in 1.9.4 when running sudo -i as an unknown user. This is related to but distinct from Bug #948.

    If the lecture_file setting is enabled in sudoers, it must now refer to a regular file or a symbolic link to a regular file.

    Fixed a potential use-after-free bug in sudo_logsrvd when the server shuts down if there are existing connections from clients that are only logging events and not session I/O data.

    Fixed a buffer size mismatch when serializing the list of IP addresses for configured network interfaces. This bug is not actually exploitable since the allocated buffer is large enough to hold the list of addresses.

    If sudo is executed with a name other than sudo or sudoedit, it will now fall back to sudo as the program name. This affects warning, help and usage messages as well as the matching of Debug lines in the /etc/sudo.conf file. Previously, it was possible for the invoking user to manipulate the program name by setting argv[0] to an arbitrary value when executing sudo.

    Sudo now checks for failure when setting the close-on-exec flag on open file descriptors. This should never fail but, if it were to, there is the possibility of a file descriptor leak to a child process (such as the command sudo runs).

    Fixed CVE-2021-23239, a potential information leak in sudoedit that could be used to test for the existence of directories not normally accessible to the user in certain circumstances. When creating a new file, sudoedit checks to make sure the parent directory of the new file exists before running the editor. However, a race condition exists if the invoking user can replace (or create) the parent directory. If a symbolic link is created in place of the parent directory, sudoedit will run the editor as long as the target of the link exists. If the target of the link does not exist, an error message will be displayed. The race condition can be used to test for the existence of an arbitrary directory. However, it cannot be used to write to an arbitrary location.

    Fixed CVE-2021-23240, a flaw in the temporary file handling of sudoedit's SELinux RBAC support. On systems where SELinux is enabled, a user with sudoedit permissions may be able to set the owner of an arbitrary file to the user-ID of the target user. On Linux kernels that support protected symlinks setting /proc/sys/fs/protected_symlinks to 1 will prevent the bug from being exploited. For more information, see Symbolic link attack in SELinux-enabled sudoedit.

    Added writability checks for sudoedit when SELinux RBAC is in use. This makes sudoedit behavior consistent regardless of whether or not SELinux RBAC is in use. Previously, the sudoedit_checkdir setting had no effect for RBAC entries.

    A new sudoers option selinux can be used to disable sudo's SELinux RBAC support.

    Quieted warnings from PVS Studio, clang analyzer, and cppcheck. Added suppression annotations for PVS Studio false positives. 

Includes security fixes for CVE-2021-23239 and CVE-2020-23240. Additional security information (from oss-security):

Hello list,

concerns have been raised with us about half a year ago that new
features like the python plugin in sudo [1] major version 1.9 could have
introduced new security issues. Recently I performed a review of the
current sudo code base and found a couple of minor and moderate issues
(not necessarily in the new features) that will be addressed in a soon
to be available sudo upstream release 1.9.5.

The following findings are all based on the `SUDO_1_9_4` tag in the
Mercurial upstream repository [2]. Only the two issues c) and d) have
been considered severe enough to request CVEs for them.

# a) User Could Enable Debug Settings not Intended for it

Function `sudo_conf_debug_files_v1()` is passed the unfiltered program
basename from `argv[0]`. In /etc/sudo.conf debug settings are based on
the program name, for example:

```
Debug sudo /var/log/sudo.log all@debug
```

Example scenario: An Admin experimented with something, say, the
`python_plugin.so` and enabled debugging for it and afterwards removed
the `python_plugin.so` from the Plugin configuration but forgot to also
remove it from the Debug configuration.

Now an unprivileged user can set argv[0] to `python_plugin.so` when
executing sudo, which will cause debugging to be enabled for the sudo
main program.

Log files created by sudo have root:root 0600 permissions so nothing
really bad should come from this. Still I think that the unprivileged
user should not be able to confuse the debugging system.

This has been addressed in upstream commit 12797:1d32c53859f9 [3].

# b) Result of `fcntl(..., FD_CLOEXEC)` is not Checked in `sudo_debug_new_output()`

In `sudo_debug.c:183` the result of `fcntl(output->fd, F_SETFD,
FD_CLOEXEC)` is ignored. If this would fail (while unlikely), then the
open debug file descriptor would be inherited into the target user
context. There seems to be no other code in place that closes this file
descriptor before executing the target command.

This has been addressed in upstream commit 12798:f1ca39a0d870 [4].

# c) CVE-2021-23239: Possible Dir Existence Test due to Race Condition in `sudoedit`

The `sudoedit` personality by default wants to prevent that the edited
file is in any way under control of an unprivileged user. This logic is
rooted in `sudo_edit_open()` / `sudo_edit_open_nonwritable()`. It
follows the complete file path from the file system root downwards and
avoids symlinks in directories that are writable by unprivileged users.

There is a corner case, however, when the target file does not exist
yet. This is handled in `sudo_edit.c:545`. `errno` will be set to
`ENOENT`, because the file didn't exist yet. Now the code checks the
parent directory of the path for existence and whether it is a
directory. If this is both true then the edit operation continues in the
expectation that later on a new file will be created. The check is done
using `stat()`, however, thus if the parent directory is under control
of the unprivileged user, it can try to win a race condition and place
an arbitrary symlink at the parent directory location just in time for
the check in `sudo_edit.c:549` to succeed.

This means the precondition covered in `sudo_edit.c:576` is no longer
true ("editing files in a writable directory is not permitted"). As far
as I can see this only allows an attacker to test for existence of
directories in arbitrary locations, if the target user is root, because
`sudoedit` behaves differently if the link target exists and is a
directory, or if it doesn't exist or isn't a directory. It *cannot* be
used to write to arbitrary locations, because the write operation
happens in `sudo_edit.c:1043` via `sudo_edit_copy_tfiles()`, which uses
`sudo_edit_open()`, this time with `O_CREAT` to open the target file.
This will not follow a symlink this time.

Example: A regular user 'testuser' is for some reason allowed to edit
the file /home/testuser/subdir/file with root privileges and without
password entry.

```
sudoedit ~/subdir/file
```

Initially ~/subdir is empty or doesn't exist. The logic in
`sudo_edit.c:545` will come into play. 'testuser' wins the race to
create a symlink:

```
ln -s /root/.gnupg ~/subdir
```

If /root/.gnupg exists then `sudoedit` will now open the editor, if it
doesn't exist it will fail with

```
sudoedit: /home/testuser/subdir/file: No such file or directory
```

This has been addressed in upstream commit 12799:ea19d0073c02 [5]

# d) CVE-2021-23240: Possible Symlink Attack in SELinux Context in `sudoedit`

If SELinux is enabled on a system then `sudoedit` uses alternate code
paths to create temporary files and to copy temporary files to target
files, namely `selinux_edit_copy_tfiles()` and
`selinux_edit_create_tfiles()`. Both functions employ `chown()` system
calls which follow symlinks.

Especially in `selinux_edit_copy_tfiles()` a `chown()` to the target
user is performed on a temporary file path that is owned by the
unprivileged user in e.g. /var/tmp. The unprivileged user could remove
this file and replace it by a symlink to a another file, that would be
followed by `sudoedit` to change its ownership.

When SELinux is in enforce mode then it should prevent such a thing to
happen. But a system might run in SELinux permissive mode in which case
the SELinux logic in `sudoedit` would still trigger but the protection
effect would be gone. In this case still the symlink protection in the
kernel can prevent the attack, if it is enabled.

This has been addressed in upstream commit 12800:8fcb36ef422a [6].

# e) Bad Buffer Size Calculation in `get_net_ifs()`

In the `get_net_ifs()` function the remaining space in the `cp` buffer
is calculated for passing it to the `snprintf()` function calls in line
175 and line 192. The calculation `ailen - (*addrinfo - cp)` is
erroneous, however, because the expression in parantheses will become
negative for increasing values of `cp`, thus passing an ever larger
buffer size to `snprintf` instead of the correctly remaining space. The
correct calculation would be `ailen - (cp - *addrinfo)`.

The impact could be a heap buffer overflow for certain values of IP
addresses on network interfaces that would exhaust the actually
available space in the `cp` buffer. However it should not be possible to
trigger this, because the buffer is allocated with enough space for
(`2 * INET6_ADDRSTRLEN`) bytes for each pair of interface address and
netmask. And even then an unprivileged user should not usually be able
to assign crafted IP addresses that would result in such an overflow.

This has been addressed in upstream commit 12796:b0cae3ac8e46 [7].

# f) Python Plugin `_verify_import()` Follows Symlinks

In `python_importblocker.c:39` a `stat()` system call that follows
symlinks is performed to determine the security of the to-be-imported
Python module. If the target directory would be under control of an
unprivileged user then it could attempt to place a symlink at the
`file_path` location that points to a file that fulfills the necessary
conditions and then could replace the symlink by a user controlled
module that would then be loaded by the Python importer.

To be completely safe here a check of all path components like done in
`sudo_edit_open_nonwritable()` would need to be made to make sure that
no unprivileged user has control over parent directories of the Python
module path.

Ideally a safely opened file descriptor would be used directly to load
the module (if possible with the Python API).

Upstream told me that this code is not actually intended to be a
security check but more of a debugging utility for admins. The Python
API does not allow to make this particular check safe. Therefore a safe
configuration is the responsibility of the sudo administrator.

# Upstream Communication

I shared this report with the sudo main developer Todd Miller on
2020-12-21. Since then we discussed the issues and possible patches, I
requested CVEs from Mitre for issues c) and d) and this week the 1.9.5
release with all bugfixes will be made.

See also the detailed analysis of issue d) done by Todd [8].

[1]: https://www.sudo.ws/
[2]: https://www.sudo.ws/repos/sudo
[3]: https://www.sudo.ws/repos/sudo/rev/1d32c53859f9
[4]: https://www.sudo.ws/repos/sudo/rev/f1ca39a0d870
[5]: https://www.sudo.ws/repos/sudo/rev/ea19d0073c02 
[6]: https://www.sudo.ws/repos/sudo/rev/8fcb36ef422a 
[7]: https://www.sudo.ws/repos/sudo/rev/b0cae3ac8e46 
[8]: https://www.sudo.ws/alerts/sudoedit_selinux.html

Cheers

Matthias

[Douglas R. Reno]

Fixed at r24109