On 27/04/2022 16:58, Carlo Arenas wrote:
On Wed, Apr 27, 2022 at 5:30 AM Phillip Wood <phillip.wood123@xxxxxxxxx> wrote:
You are ignoring any errors when parsing the environment variable - that
is not a good idea in a security check.
which errors are you concerned about?, if anything in this code
I was confused by the fact that the helper function returns a
success/failure value which we ignore. However euid is not overwritten
if strtol fails so it is safe I think.
worries me from a security point of view is the fact that we are
relying in getenv not being racy (as mentioned in the original RFC),
but there are no errors set there AFAIK.
not ignoring errno in strtol is an option, but as mentioned before I
decided instead to reject bogus values and therefore not the clobber a
previous errno,
strtol() will set errno if there is a range error ignoring it does not
change that. In any case is_path_owned_by_current_uid() already clobbers
errno if stat() fails.
Best Wishes
Phillip
since I was using strtol as a wider version of atoi.
Carlo
