On 27/04/2022 16:38, Carlo Arenas wrote:
On Wed, Apr 27, 2022 at 2:33 AM Phillip Wood <phillip.wood123@xxxxxxxxx> wrote:
On 27/04/2022 01:05, Carlo Marcelo Arenas Belón wrote:
[...]
FWIW, I still think that using atoi with a check to skip "" is
probably as safe as doing all this extra checking as no one has shown
yet a system where sizeof(uid_t) > sizeof(uint32_t), but agree with
Junio that using long instead avoids issues with the systems where
sizeof(uid_t) > sizeof(int) and unless sizeof(int) == sizeof(long)
(ex: 32-bit Linux) which is then covered by the cast.
if sizeof(uid_t) < sizeof(long) then the cast will truncate the value
returned by strtol() which means we are trusting that SUDO_UID is a
valid uid otherwise it will be truncated.
+{
+ 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 last two checks were to check for underflow or overflow and to
make sure that the bogus values this function returns in case of those
errors are not taken into consideration.
Sorry I misread the code - this is checking if the value is valid, not
if there is an error. On platforms where uid_t and long are the same
size (e.g. where int and long are both 32 bits) the "< LONG_MAX" check
is rejecting a valid uid. The fact that we are doing these checks makes
me think we should care about the possible truncation above.
Best Wishes
Phillip
The standard is silent on what happens to error when the value is out of
range.
Which is why instead I was avoiding LONG_{MIN,MAX} which are
described[1] as the bogus values that will be used in that case
Agree with you that we could also add a check for >=0 as uid_t is
usually unsigned, but it is not warranted to be so, and that is I was
aiming for the wider possible range so we don't have to worry that
much and let it be settled to a valid value through the cast.
Carlo
[1] https://pubs.opengroup.org/onlinepubs/007904875/functions/strtol.html
