diff options
author | Julius Werner <jwerner@chromium.org> | 2019-03-22 13:03:41 -0700 |
---|---|---|
committer | Patrick Georgi <pgeorgi@google.com> | 2019-03-26 11:25:41 +0000 |
commit | 78134b0ccd752f1d7029058a24ba103ac31cfdd5 (patch) | |
tree | be872b3d04f6f566507963a52e7b5d7ff67ebdc1 /payloads | |
parent | 413f5208f0688fb0f44dc3bca0259d1050b254f0 (diff) |
libpayload: strtoull: Fix edge case bug with *endptr
strtoull() can optionally take a second pointer as an out-parameter that
will be adjusted to point to the end of the parsed string. This works
almost right, but misses two important edge cases: firstly,when the
parsed string is "0", the function will interpret the leading '0' as an
octal prefix, so that the first actually parsed digit is already the
terminating '\0' byte. This will cause the function to early abort,
which still (correctly) returns 0 but doesn't adjust *endptr.
The early abort is pointless anyway -- the only other thing the function
does is run a for-loop whose condition is the exact inverse (so it's
guaranteed to run zero iterations in this case) and then adjust *endptr
(which we want). So just take it out. This also technically corrects the
behavior of *endptr for a completely invalid string, since the strtoull
man page says
> If there were no digits at all, strtoul() stores the original value of
> nptr in *endptr (and returns 0).
The second issue occurs when the parsed string is "0x" without another
valid digit behind it. In this case, we will still jump over the 0x
prefix so that *endptr is set to the first byte after that. The correct
interpretation in this case is that there is no 0x prefix, and instead a
valid 0 digit with the 'x' being invalid garbage at the end. By not
skipping the prefix unless there's at least one valid digit after it, we
get the correct behavior of *endptr pointing to the 'x'.
Change-Id: Idddd74e18e410a9d0b6dce9512ca0412b9e2333c
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32029
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Diffstat (limited to 'payloads')
-rw-r--r-- | payloads/libpayload/libc/string.c | 9 |
1 files changed, 2 insertions, 7 deletions
diff --git a/payloads/libpayload/libc/string.c b/payloads/libpayload/libc/string.c index a58efde135..f563f6338a 100644 --- a/payloads/libpayload/libc/string.c +++ b/payloads/libpayload/libc/string.c @@ -517,16 +517,11 @@ unsigned long long int strtoull(const char *ptr, char **endptr, int base) /* Base 16 allows the 0x on front - so skip over it */ if (base == 16) { - if (ptr[0] == '0' && (ptr[1] == 'x' || ptr[1] == 'X')) + if (ptr[0] == '0' && (ptr[1] == 'x' || ptr[1] == 'X') && + _valid(ptr[2], base)) ptr += 2; } - /* If the first character isn't valid, then don't - * bother */ - - if (!*ptr || !_valid(*ptr, base)) - return 0; - for( ; *ptr && _valid(*ptr, base); ptr++) ret = (ret * base) + _offset(*ptr, base); |