From a48d54dfc5a7779e1d8d0e0b4f4b3064c9a403f7 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Wed, 24 Feb 2021 14:33:50 +0100 Subject: [PATCH] Kernel: Don't dereference untrusted userspace pointer in sys$uname() Instead of writing to the userspace utsname struct one field at a time, build up a utsname on the kernel stack and copy it out to userspace once it's finished. This is both simpler and gets validity checking built-in for free. Found by KUBSAN! :^) Fixes #5499. --- Kernel/Syscalls/uname.cpp | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/Kernel/Syscalls/uname.cpp b/Kernel/Syscalls/uname.cpp index 6f56388396..3b2279653c 100644 --- a/Kernel/Syscalls/uname.cpp +++ b/Kernel/Syscalls/uname.cpp @@ -39,18 +39,14 @@ int Process::sys$uname(Userspace buf) if (g_hostname->length() + 1 > sizeof(utsname::nodename)) return -ENAMETOOLONG; - // We have already validated the entire utsname struct at this - // point, there is no need to re-validate every write to the struct. - utsname* user_buf = buf.unsafe_userspace_ptr(); - if (!copy_to_user(user_buf->sysname, "SerenityOS", 11)) - return -EFAULT; - if (!copy_to_user(user_buf->release, "1.0-dev", 8)) - return -EFAULT; - if (!copy_to_user(user_buf->version, "FIXME", 6)) - return -EFAULT; - if (!copy_to_user(user_buf->machine, "i686", 5)) - return -EFAULT; - if (!copy_to_user(user_buf->nodename, g_hostname->characters(), g_hostname->length() + 1)) + utsname buf {}; + memcpy(buf.sysname, "SerenityOS", 11); + memcpy(buf.release, "1.0-dev", 8); + memcpy(buf.version, "FIXME", 6); + memcpy(buf.machine, "i686", 5); + memcpy(buf.nodename, g_hostname->characters(), g_hostname->length() + 1); + + if (!copy_to_user(user_buf, &buf)) return -EFAULT; return 0; }