Date: Mon, 11 Jan 2021 15:05:23 +0100 From: Matthias Gerstner <mgerstner@...e.de> To: oss-security@...ts.openwall.com Subject: Various security fixes in sudo 1.9.5 (CVE-2021-23239, CVE-2021-23240) Hello list, concerns have been raised with us about half a year ago that new features like the python plugin in sudo  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 . 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`. In /etc/sudo.conf debug settings are based on the program name, for example: ``` Debug sudo /var/log/sudo.log all@...ug ``` 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 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 . # 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 . # 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  # 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 . # 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 . # 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 . : https://www.sudo.ws/ : https://www.sudo.ws/repos/sudo : https://www.sudo.ws/repos/sudo/rev/1d32c53859f9 : https://www.sudo.ws/repos/sudo/rev/f1ca39a0d870 : https://www.sudo.ws/repos/sudo/rev/ea19d0073c02 : https://www.sudo.ws/repos/sudo/rev/8fcb36ef422a : https://www.sudo.ws/repos/sudo/rev/b0cae3ac8e46 : https://www.sudo.ws/alerts/sudoedit_selinux.html Cheers Matthias -- Matthias Gerstner <matthias.gerstner@...e.de> Dipl.-Wirtsch.-Inf. (FH), Security Engineer https://www.suse.com/security Phone: +49 911 740 53 290 GPG Key ID: 0x14C405C971923553 SUSE Software Solutions Germany GmbH HRB 36809, AG Nürnberg Geschäftsführer: Felix Imendörffer Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists
Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.