| Summary: | dehumanize_number() doesn't detect range errors between INT64_MAX and UINT64_MAX for int64_t result | ||
|---|---|---|---|
| Product: | libbsd | Reporter: | C <cse.cem+bugsfreedesktoporg> |
| Component: | libbsd | Assignee: | Guillem Jover <guillem> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | medium | ||
| Version: | unspecified | ||
| Hardware: | Other | ||
| OS: | All | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
Hi!
Nicely spotted. Although I don't think the correction is right. I've changed the code to look something like this instead:
uint64_t rval, rmax;
rmax = INT64_MAX + 1ULL;
if (rval > rmax || (rval == rmax && sign == 1)) {
errno = ERANGE;
return -1;
}
And updated the test suite. Will be included in 0.6.1, thanks.
(In reply to comment #1) > Hi! > > Nicely spotted. Although I don't think the correction is right. Yeah, I got the ends of two's complement signed integers wrong, oops! Just goes to show how easy it is to mess up. > I've changed > the code to look something like this instead: > > uint64_t rval, rmax; > > rmax = INT64_MAX + 1ULL; > if (rval > rmax || (rval == rmax && sign == 1)) { > errno = ERANGE; > return -1; > } > > And updated the test suite. Will be included in 0.6.1, thanks. The "&& sign == 1" in this correction wasn't intuitive for me, but after reviewing it I believe it is correct. Thanks! |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.
==================================8<================================= dehumanize_number(const char *buf, int64_t *num) { uint64_t rval; ... rc = expand_number(buf, &rval); ... if (rval == UINT64_MAX && sign == -1) { errno = ERANGE; return -1; } *num = rval * sign; ... ==================================8<================================= The return value is a signed 64-bit int, so any value less than -(INT64_MAX - 1) or greater than INT64_MAX is an overflow. The range checking code could look more like this: if (rval > INT64_MAX) { errno = ERANGE; return -1; } else if (rval == INT64_MAX && sign == -1) { errno = ERANGE; return -1; }