"Phillip Wood via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> From: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
>
> git_parse_signed() checks that the absolute value of the parsed string
> is less than or equal to a caller supplied maximum value. When
> calculating the absolute value there is a integer overflow if `val ==
> INTMAX_MIN`.
It is a problem that is worth looking into.
> Signed-off-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
> ---
> config.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/config.c b/config.c
> index b7fb68026d8..aad3e00341d 100644
> --- a/config.c
> +++ b/config.c
> @@ -1160,8 +1160,10 @@ static int git_parse_signed(const char *value, intmax_t *ret, intmax_t max)
> if (value && *value) {
> char *end;
> intmax_t val;
> - uintmax_t uval;
> - uintmax_t factor;
> + intmax_t factor;
> +
> + if (max < 0)
> + BUG("max must be a positive integer");
In parse_signed(), would we expect to accept end-user input that is
a negative integer? We must. Otherwise we would not be calling a
"signed" parser. Now, are there cases where the valid value range
is bounded by a negative integer at the top? No current callers may
pass such a value, but is it reasonable to add such a new constraints
to an existing API function?
