improved integer overflow fixes #331
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* ch.c (ch_setbufspace): Do not overflow when INT_MAX / 8 < bufspace.
* os.c (sleep_ms): Do not overflow when INT_MAX - 999 < ms.
This doesn't change behavior; it merely does some refactoring that will be useful in future changes. * lesskey.c, main.c (out_of_memory): New function. (ecalloc): Use it.
Use C23 ckd_add and ckd_mul macros to check for integer overflow without relying on undefined behavior. On pre-C23 platforms, fall back on substitutes that are good enough for 'less'. Although this patch does not address all such bugs, it captures much of the low-hanging fruit. * charset.c (ichardef): * decode.c (getcc_int): * mark.c (restore_mark): * optfunc.c (set_tabs): * option.c (getnum): * screen.c (ltputs, parse_color6): * xbuf.c (xbuf_add_byte): Check for integer overflow. * cmdbuf.c (cmd_int): Do not rely on undefined behavior when checking for integer overflow. * configure.ac: Check for C99's inttypes.h and C23's stdckdint.h. * less.h: Include stdint.h and stdckdint.h if available. (ckd_add, ckd_mul): If not defined, define adequate substitutes. (signed_expr) [!HAVE_STDCKDINT_H]: New helper macro. (uintmax): New typedef. * option.c (num_error): New arg if caller detected overflow. All callers changed. (getfraction): Redo fraction calculation to avoid integer overflow. * output.c (STR_TO_TYPE_FUNC, lstrtops, lstrtoi, lstrtoul): Return ((type) -1) on integer overflow. * xbuf.c (help_fixup, help_ckd_add, help_ckd_mul): New functions.
* less.h: Comment on fraction ranges. * optfunc.c (calc_jump_sline, calc_shift_count): Use muldiv to avoid integer overflow. * os.c (muldiv): Now public, and operates on uintmax values. Use only integer arithmetic, since using 'double' can yield rounding errors. This is safe given that callers alreasy arrange for min(VAL, NUM) <= DEN so the mathematical result must fit. (percent_pos): Do not divide by 100 first, as that introduces a rounding error, and the division is not necessary to avoid overflow. Do not special-case zero, as muldiv handles that now.
'less' no longer uses 'double' anywhere, so there's no longer any need for HAVE_FLOAT or './configure --no-float'. * configure.ac, defines.ds, defines.wn, os.c (HAVE_FLOAT): Remove.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I looked at PR #243 and was inspired to write this new pull request, which does a better job of integer overflow detection. This PR does not fix all the integer overflow issues in
lessbut I hope it catches most of the likely ones. This PR uses the following ideas:ckd_addandckd_mulmacros that will be standardized in C23's<stdckdint.h>, and fall back on portable substitutes good enough forlesson platforms that don't yet support these macros (which is most platforms today). This is because in the long run it'll be better to use the standard way of checking for integer overflow than to use idiosyncratic macros or functions.double, so thatHAVE_FLOATis no longer needed.I think this PR does everything that PR #243 does, so if you install it you shouldn't need to install PR #243.