Overflow in `parse_size`

[Smattr]

When calling strtod, it seems to me there is no obligation for the implementation to set the pointer in the second argument if the conversion would overflow. To quote the relevant man page:

If the correct value would cause overflow, plus or minus HUGE_VAL (HUGE_VALF, HUGE_VALL) is returned (according to the sign of the value), and ERANGE is stored in errno. If the correct value would cause underflow, zero is returned and ERANGE is stored in errno.

So if parse_size receives an input that is larger than the largest representable double, strtod returns HUGE_VAL and parse_size goes on to reference an uninitialised pointer (https://github.com/hoytech/vmtouch/blob/master/vmtouch.c#L278). In practice, this doesn't seem to be an issue with Glibc, which does initialise the pointer in this failure scenario, however I don't think it's under any obligation to do this.

It also seems like the final multiplication in this function, mult * val, can overflow without being detected.

I'm not intimately familiar with the code of vmtouch, so I may be mistaken. If so, apologies for the noise or please let me know if anything I've said is unclear. Thanks.

[hoytech]

Thanks for the report. Interesting point about strtod not setting endptr.

My reading of the standards and man-pages is that endptr should always be set (unless it is NULL). Do you have any references to where it wouldn't be set?

Yeah it's an interesting case when users pass in crazy large values. I believe usually they'll be returned as floating point INFINITY in which case the multiplication's result will fall outside the acceptable range and thus error out.

I think it would be worthwhile adding some checks for the infinity/nan cases.. Usually this would be a user fat-fingering something so we may as well give a nicer error message. I'll think about this.

Also, btw, while I was checking this out I noticed another little usage problem: #27

Cheers,

Doug

[Smattr]

I have nothing to go on except the man page, and re-reading it now I think I may be wrong.

RETURN VALUE These functions return the converted value, if any.

If endptr is not NULL, a pointer to the character after the last character used in the conversion is stored in the location referenced by endptr.

If no conversion is performed, zero is returned and the value of nptr is stored in the location referenced by endptr.

If the correct value would cause overflow, plus or minus HUGE_VAL (HUGE_VALF, HUGE_VALL) is returned (according to the sign of the value), and ERANGE is stored in errno. If the correct value would cause underflow, zero is returned and ERANGE is stored in errno.

I had interpreted the second, third and fourth paragraphs as representing distinct cases, but now I'm thinking perhaps the second is meant to apply in all cases. POSIX has this to say, which backs up this second interpretation:

If the subject sequence is empty or does not have the expected form, no conversion shall be performed; the value of str is stored in the object pointed to by endptr, provided that endptr is not a null pointer.

Reading the docs for strtod reveal some pretty weird behaviour (e.g. you can pass in the string "inf" or "nan").

To be doubly safe (or paranoid, depending on your perspective) the comparison of val could also test == HUGE_VAL. I can put together a pull request that does this if you think it's the correct way to go.

[hoytech]

Sure I'd check out a pull request, thanks!