KSPP / linux

Linux kernel source tree (Kernel Self Protection Project)
https://kernsec.org/wiki/index.php/Kernel_Self_Protection_Project
Other
82 stars 5 forks source link

Enforce argv!=NULL to avoid confused userspace argv iteration when argc is assumed to always be >0 #176

Closed kees closed 2 years ago

kees commented 2 years ago

Calls of execve(..., NULL, ...) should be rejected by the kernel. It's nonsense and was used in a recent attack: https://www.qualys.com/2022/01/25/cve-2021-4034/pwnkit.txt

This was reported back in 2008, too: https://bugzilla.kernel.org/show_bug.cgi?id=8408

It should be trivial to fix, though there may be some corner cases that don't like it, https://codesearch.debian.net/search?q=execve%5C+*%5C%28%5B%5E%2C%5D%2B%2C+*NULL&literal=0

such as valgrind's test suite: https://sources.debian.org/src/valgrind/1:3.18.1-1/none/tests/execve.c/?hl=22#L22

For the patch thread, see: https://lore.kernel.org/lkml/20220126114447.25776-1-ariadne@dereferenced.org

kees commented 2 years ago

https://lore.kernel.org/lkml/YfE%2FowUY+gVnn2b%2F@selene.zem.fi points out execlp() is also a potential problem: https://codesearch.debian.net/search?q=execlp%3F%5Cs*%5C%28%5B%5E%2C%5D%2B%2C%5Cs*NULL&literal=0

kees commented 2 years ago

If anything depends on argc == 0, we can't inject a "default" argv. e.g.:

execve(path, { path, NULL }, ...)

Note that execlp() will pass through the existing envp, so checking for envc != 0 may similarly run aground.

Hello71 commented 2 years ago

https://lore.kernel.org/lkml/YfE/owUY+gVnn2b/@selene.zem.fi

https://lore.kernel.org/lkml/YfE%2FowUY+gVnn2b%2F@selene.zem.fi

kees commented 2 years ago

https://lore.kernel.org/lkml/YfE/owUY+gVnn2b/@selene.zem.fi

https://lore.kernel.org/lkml/YfE%2FowUY+gVnn2b%2F@selene.zem.fi

Agh slashes. Thanks! I've updated the original comment. :)

DemiMarie commented 2 years ago

What about argv[0] != NULL && argv[0][0] == '\0'? That also seems like nonsense, inasmuch as the empty string is not a valid file name (AT_EMPTY_PATH isn’t applicable here).

kees commented 2 years ago

The kernel walks the passed in argv list to count the arguments for itself. So argv = NULL and argv = { NULL } are identical. Having argv = { "", NULL } means there is actually a NULL pointer at argv[1], which is fine. That solves the primary weakness in userspace programs that assume argc must start at 1 (i.e. they start walking envp as arguments).

kees commented 2 years ago

Newest attempt: https://lore.kernel.org/lkml/20220201000947.2453721-1-keescook@chromium.org

kees commented 2 years ago

Landed with commit dcd46d897adb70d63e025f175a00a89797d31a43. Selftest is commit 9132c3947b09a6c67372424ff69f867f2cee82f8.