Kernel: Return EINVAL when specifying -1 for setuid and similar syscalls

For setreuid and setresuid syscalls, -1 means to set the current
uid/euid/gid/egid value, to be more convenient for programming.
However, for other syscalls where we pass only one argument, there's no
justification to specify -1.

This behavior is identical to how Linux handles the value -1, and is
influenced by the fact that the manual pages for the group of one
argument syscalls that handle ID operations is ambiguous about this
topic.
This commit is contained in:
Liav A 2021-12-19 15:10:45 +02:00 committed by Andreas Kling
parent a3c732b8ae
commit 5a649d0fd5
Notes: sideshowbarker 2024-07-17 22:34:17 +09:00
5 changed files with 43 additions and 0 deletions

View File

@ -27,6 +27,7 @@ Otherwise, returns -1 and sets `errno` to describe the error.
## Errors
* `EPERM`: The new ID is not equal to the real ID or saved ID, and the user is not superuser.
* `EINVAL`: The new ID is set to invalid value (-1).
## See also

View File

@ -25,6 +25,7 @@ Otherwise, returns -1 and sets `errno` to describe the error.
## Errors
* `EPERM`: The new ID is not equal to the real ID or effective ID, and the user is not superuser.
* `EINVAL`: The new ID is set to invalid value (-1).
## See also

View File

@ -13,6 +13,9 @@ ErrorOr<FlatPtr> Process::sys$seteuid(UserID new_euid)
VERIFY_PROCESS_BIG_LOCK_ACQUIRED(this)
REQUIRE_PROMISE(id);
if (new_euid == (uid_t)-1)
return EINVAL;
if (new_euid != uid() && new_euid != suid() && !is_superuser())
return EPERM;
@ -30,6 +33,9 @@ ErrorOr<FlatPtr> Process::sys$setegid(GroupID new_egid)
VERIFY_PROCESS_BIG_LOCK_ACQUIRED(this)
REQUIRE_PROMISE(id);
if (new_egid == (uid_t)-1)
return EINVAL;
if (new_egid != gid() && new_egid != sgid() && !is_superuser())
return EPERM;
@ -46,6 +52,9 @@ ErrorOr<FlatPtr> Process::sys$setuid(UserID new_uid)
VERIFY_PROCESS_BIG_LOCK_ACQUIRED(this)
REQUIRE_PROMISE(id);
if (new_uid == (uid_t)-1)
return EINVAL;
if (new_uid != uid() && new_uid != euid() && !is_superuser())
return EPERM;
@ -64,6 +73,9 @@ ErrorOr<FlatPtr> Process::sys$setgid(GroupID new_gid)
VERIFY_PROCESS_BIG_LOCK_ACQUIRED(this)
REQUIRE_PROMISE(id);
if (new_gid == (uid_t)-1)
return EINVAL;
if (new_gid != gid() && new_gid != egid() && !is_superuser())
return EPERM;

View File

@ -33,6 +33,7 @@ serenity_test("crash.cpp" Kernel MAIN_ALREADY_DEFINED)
set(LIBTEST_BASED_SOURCES
TestEFault.cpp
TestInvalidUIDSet.cpp
TestKernelAlarm.cpp
TestKernelFilePermissions.cpp
TestKernelPledge.cpp

View File

@ -0,0 +1,28 @@
/*
* Copyright (c) 2021, Liav A. <liavalb@hotmail.co.il>
*
* SPDX-License-Identifier: BSD-2-Clause
*/
#include <LibTest/TestCase.h>
#include <errno.h>
#include <unistd.h>
TEST_CASE(test_invalid_set_uid_parameters)
{
auto res = setuid(-1);
EXPECT_EQ(res, -1);
EXPECT_EQ(errno, EINVAL);
res = seteuid(-1);
EXPECT_EQ(res, -1);
EXPECT_EQ(errno, EINVAL);
res = setgid(-1);
EXPECT_EQ(res, -1);
EXPECT_EQ(errno, EINVAL);
res = setegid(-1);
EXPECT_EQ(res, -1);
EXPECT_EQ(errno, EINVAL);
}