apply: detect overflow when parsing hunk header

[phillipwood]

We should do something similar in "git add -p" but I'll wait to see what happens with https://lore.kernel.org/git/[email protected]/ first

Cc: Sören Krecker [email protected]

[phillipwood]

/submit

[gitgitgadget]

Submitted as [email protected]

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1858/phillipwood/apply-detect-hunk-header-overflow-v1

To fetch this version to local tag pr-1858/phillipwood/apply-detect-hunk-header-overflow-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1858/phillipwood/apply-detect-hunk-header-overflow-v1

[gitgitgadget]

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Phillip Wood via GitGitGadget" <[email protected]> writes:

> From: Phillip Wood <[email protected]>
>
> "git apply" uses strtoul() to parse the numbers in the hunk header but
> silently ignores overflows. As LONG_MAX is a legitimate return value for
> strtoul() we need to set errno to zero before the call to strtoul() and
> check that it is still zero afterwards. The error message we display is
> not particularly helpful as it does not say what was wrong.  However, it
> seems pretty unlikely that users are going to trigger this error in
> practice and we can always improve it later if needed.

Thanks.  We made an effort to use a type that is a bit wider than
"int", but we apparently ignored that the Git userbase will become a
lot wider some day and unfriendly and/or hostile folks would start
feeding malicious input to us X-<.  The check presented here look
good, and the fact that there was only one change needed shows how
well designed the base code was ;-)

Will queue.  Thanks.

> @@ -1423,7 +1423,10 @@ static int parse_num(const char *line, unsigned long *p)
>  
>  	if (!isdigit(*line))
>  		return 0;
> +	errno = 0;
>  	*p = strtoul(line, &ptr, 10);
> +	if (errno)
> +		return 0;
>  	return ptr - line;
>  }
>  
> diff --git a/t/t4100-apply-stat.sh b/t/t4100-apply-stat.sh
> index 146e73d8f55..a5664f3eb3c 100755
> --- a/t/t4100-apply-stat.sh
> +++ b/t/t4100-apply-stat.sh
> @@ -38,4 +38,17 @@ incomplete (1)
>  incomplete (2)
>  EOF
>  
> +test_expect_success 'applying a hunk header which overflows fails' '
> +	cat >patch <<-\EOF &&
> +	diff -u a/file b/file
> +	--- a/file
> +	+++ b/file
> +	@@ -98765432109876543210 +98765432109876543210 @@
> +	-a
> +	+b
> +	EOF
> +	test_must_fail git apply patch 2>err &&
> +	echo "error: corrupt patch at line 4" >expect &&
> +	test_cmp expect err
> +'
>  test_done
>
> base-commit: fbe8d3079d4a96aeb4e4529cc93cc0043b759a05

[gitgitgadget]

This patch series was integrated into seen via git@c5f2e07.