Implement it as an inline function, and add restrict and ATTR_STRING()
and ATTR_ACCESS() as appropriate.
Reviewed-by: "Serge E. Hallyn" <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
'endptr' is appropriate internally in strtol(3) because it's a pointer
to 'end', and 'end' itself is a pointer to one-after-the-last character
of the numeric string. In other words,
endptr == &end
However, naming the pointer whose address we pass to strtol(3)'s
'endptr' feels wrong, and causes me trouble while parsing the code; I
need to double check the number of dereferences, because something feels
wrong in my head.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
The variable declarations for the buffers have been aligned in this
commit, so that they appear in the diff, making it easier to review.
Some important but somewhat tangent changes included in this commit:
- lib/nss.c: The size was being defined as 65, but then used as 64.
That was a bug, although not an important one; we were just wasting
one byte. Fix that while we replace snprintf() by SNPRINTF(), which
will get the size from sizeof(), and thus will use the real size.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
- Set errno = 0 before the call. Otherwise, it may contain anything.
- ERANGE is not the only possible errno value of these functions. They
can also set it to EINVAL.
- Any errno value after these calls is bad; just compare against 0.
- Don't check for the return value; just errno. This function is
guaranteed to not modify errno on success (POSIX).
- Check endptr == str, which may or may not set EINVAL.
Suggested-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Neither a pid_t below 1 nor a negative fd could be valid in this context.
Proof of Concept:
$ newuidmap -1 1 1 1
newuidmap: Could not open proc directory for target 4294967295
Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
write_mapping() will do the following:
openat(proc_dir_fd, map_file, O_WRONLY);
An attacker could create a directory containing a symlink named
"uid_map" pointing to any file owned by root, and thus allow him to
overwrite any root-owned file.
Closes#635
newuidmap and newgidmap currently take an integner pid as
the first argument, determining the process id on which to
act. Accept also "fd:N", where N must be an open file
descriptor to the /proc/pid directory for the process to
act upon. This way, if you
exec 10</proc/99
newuidmap fd:10 100000 0 65536
and pid 99 dies and a new process happens to take pid 99 before
newuidmap happens to do its work, then since newuidmap will use
openat() using fd 10, it won't change the mapping for the new
process.
Example:
// terminal 1:
serge@jerom ~/src/nsexec$ ./nsexec -W -s 0 -S 0 -U
about to unshare with 10000000
Press any key to exec (I am 129176)
// terminal 2:
serge@jerom ~/src/shadow$ exec 10</proc/129176
serge@jerom ~/src/shadow$ sudo chown root src/newuidmap src/newgidmap
serge@jerom ~/src/shadow$ sudo chmod u+s src/newuidmap
serge@jerom ~/src/shadow$ sudo chmod u+s src/newgidmap
serge@jerom ~/src/shadow$ ./src/newuidmap fd:10 0 100000 10
serge@jerom ~/src/shadow$ ./src/newgidmap fd:10 0 100000 10
// Terminal 1:
uid=0(root) gid=0(root) groups=0(root)
Signed-off-by: Serge Hallyn <serge@hallyn.com>
- Every non-const pointer converts automatically to void *.
- Every pointer converts automatically to void *.
- void * converts to any other pointer.
- const void * converts to any other const pointer.
- Integer variables convert to each other.
I changed the declaration of a few variables in order to allow removing
a cast.
However, I didn't attempt to edit casts inside comparisons, since they
are very delicate. I also kept casts in variadic functions, since they
are necessary, and in allocation functions, because I have other plans
for them.
I also changed a few casts to int that are better as ptrdiff_t.
This change has triggered some warnings about const correctness issues,
which have also been fixed in this patch (see for example src/login.c).
Signed-off-by: Alejandro Colomar <alx@kernel.org>
