Address CVE-2013-4235 #545
Conversation
|
Awesome, I've been wanting to do go in the direction of using more *_at. I'll look at it closely in just a bit. |
|
Please rebase this onto master to get past the impish failure. |
Use *at() functions to pin the directory operating in to avoid being redirected by unprivileged users replacing parts of paths by symlinks to privileged files.
Use *at() functions to pin the directory operating in to avoid being redirected by unprivileged users replacing parts of paths by symlinks to privileged files.
Require lstat(2), lchown(2), S_IFLNK and S_ISLNK from POSIX.1-2001. Already unconditionally used in lib/tcbfuncs.c and lib/run_part.c.
Similar to the default behavior of mkdir(2), symlink(2), link(2) and mknod(2).
Bail out on read(2) failure, continue on EINTR, support short writes and increase chunk size.
copydir.c:666:44: warning: unsigned conversion from 'int' to '__mode_t' {aka 'unsigned int'} changes value from '-4096' to '4294963200' [-Wsign-conversion]
666 | if ( (mknod (dst, statp->st_mode & ~07777, statp->st_rdev) != 0)
| ^
copydir.c:116:1: warning: missing initializer for field 'quote' of 'struct error_context' [-Wmissing-field-initializers]
116 | };
| ^
In file included from copydir.c:27:
/usr/include/attr/error_context.h:30:23: note: 'quote' declared here
30 | const char *(*quote) (struct error_context *, const char *);
| ^~~~~
|
@brauner do you have any other comments on this PR? I'd been hoping to make a release today. Only two of my remaining comments above actually seem important, so I'm trying to decide whether to merge as is before releasing or not. |
Use *at() functions to pin the directory operating in to avoid being redirected by unprivileged users replacing parts of paths by symlinks to privileged files. Introduce a path_info struct with the full path and dirfd and name information for *at() functions, since the full path is needed for link resolution, SELinux label lookup and ACL attributes.
|
I released 4.12 today without this pr - because I have my nitrokey today and may not again for awhile. But I'm hoping to release 4.12.1 or 4.13 asap after this is merged. |
|
Thanks. I'll merge this in a few hours unless @brauner chimes in wanting to review before that. |
https://build.opensuse.org/request/show/999092 by user jubalh + dimstar_suse - Update to 4.12.3: Revert removal of subid_init, which should have bumped soname. So note that 4.12 through 4.12.2 were broken for subid users. - Update to 4.12.2: * Address CVE-2013-4235 (TOCTTOU when copying directories) [bsc#916845] - Refresh useradd-userkeleton.patch: LSTAT() was removed with shadow-maint/shadow#545 Let's use fstatat() now. - Update to 4.12.1: * Fix uk manpages - Remove shadow-4.12-remove-uk.patch: fixed upstream - Update to 4.12: * Add absolute path hint to --root * Various cleanups * Fix Ubuntu release used in CI tests * add -F options to userad * useradd manpage updates * Check for ownerid (not just username) in subid ranges * Declare file local functions static * Use strict prototypes * Do not drop const qual
Well, that was papering it over. Let's say you are deleting user joe, but user joe had a file owned by user mitch. dcca865 would force joe's processes to be killed, but user mitch might in theory be able to use the TOCTTOU to make bad things happen. This PR actually addresses the TOCTTOU itself. |
Address CVE-2013-4235 (#317) by pinning the directories operating in and using the *at() version of file related functions.
Requires openat(2), fstatat(2), mkdirat(2), symlinkat(2), linkat(2), mknodat(2), unlinkat(2), fchmodat(2), fchownat(2) and utimensat(2) specified in POSIX.1-2008.
Should be supported by glibc 2.6 and Linux 2.6.22.