From 78134b0ccd752f1d7029058a24ba103ac31cfdd5 Mon Sep 17 00:00:00 2001 From: Julius Werner Date: Fri, 22 Mar 2019 13:03:41 -0700 Subject: [PATCH] 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 Reviewed-on: https://review.coreboot.org/c/coreboot/+/32029 Tested-by: build bot (Jenkins) Reviewed-by: Nico Huber Reviewed-by: Paul Menzel --- payloads/libpayload/libc/string.c | 9 ++------- 1 file 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);