On 27/04/2022 10:33, Phillip Wood wrote:
Hi Carlo
On 27/04/2022 01:05, Carlo Marcelo Arenas Belón wrote:
[...]
+{
+ const char *real_uid = getenv(env);
+
+ if (real_uid && *real_uid) {
+ char *error;
+ long extracted_id = strtol(real_uid, &error, 10);
+ if (!*error && LONG_MIN < extracted_id &&
+ extracted_id < LONG_MAX) {
strtol() returns a long so the last two checks are redundant. The
standard is silent on what happens to error when the value is out of
range. The way to check that all the string was consumed without
underflow/overflow is
errno = 0;
val = strtol(str, &endp, 10);
if (errno || !*endp)
Sorry that should be "if (errno || *endp)" to check for an error
> [...]
> #if sizeof(uid_t) == sizeof(int)
> if (extracted_id > INT_MAX)
> error(...)
> #endif
I think we should probably check if uid_t is a short integer as well as
> +#ifdef __TANDEM
> +#define ROOT_UID 65535
suggests it may be an unsigned short on NonStop.
Not knowing the size of uid_t or if it is signed or not (as far as I can
tell posix just says it's an integer) makes the limit checks tricky -
maybe we make euid a long (or unsigned long) here and it the function
below rather than casting it to uid_t and possibly truncating it.
> [...]
+
static inline int is_path_owned_by_current_uid(const char *path)
{
struct stat st;
+ uid_t euid;
+
if (lstat(path, &st))
return 0;
- return st.st_uid == geteuid();
+
+ euid = geteuid();
+ if (euid == ROOT_UID) {
+ /* we might have raised our privileges with sudo */
+ extract_id_from_env("SUDO_UID", &euid);
You are ignoring any errors when parsing the environment variable - that
is not a good idea in a security check.
Best Wishes
Phillip
+ }
+ return st.st_uid == euid;
}
#define is_path_owned_by_current_user is_path_owned_by_current_uid
