From: René Scharfe <l.s.r@web.de>
Date: Sat, 25 Feb 2017 20:51:47 GMT
Subject: Re: [PATCH 1/2] apply: guard against renames of non-existant empty files
Message-ID: <baf195cc-ef81-bbad-4e01-4149498efedb@web.de>
In-Reply-To: <20170225101307.24067-1-vegard.nossum@oracle.com>

Am 25.02.2017 um 11:13 schrieb Vegard Nossum:
> If we have a patch like the one in the new test-case, then we will
> try to rename a non-existant empty file, i.e. patch->old_name will
> be NULL. In this case, a NULL entry will be added to fn_table, which
> is not allowed (a subsequent binary search will die with a NULL
> pointer dereference).
>
> The patch file is completely bogus as it tries to rename something
> that is known not to exist, so we can throw an error for this.
>
> Found using AFL.
>
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
> ---
>  apply.c                     |  3 ++-
>  t/t4154-apply-git-header.sh | 15 +++++++++++++++
>  2 files changed, 17 insertions(+), 1 deletion(-)
>  create mode 100755 t/t4154-apply-git-header.sh
>
> diff --git a/apply.c b/apply.c
> index 0e2caeab9..cbf7cc7f2 100644
> --- a/apply.c
> +++ b/apply.c
> @@ -1585,7 +1585,8 @@ static int find_header(struct apply_state *state,
>  				patch->old_name = xstrdup(patch->def_name);
>  				patch->new_name = xstrdup(patch->def_name);
>  			}
> -			if (!patch->is_delete && !patch->new_name) {
> +			if ((!patch->is_delete && !patch->new_name) ||
> +			    (patch->is_rename && !patch->old_name)) {

Would it make sense to mirror the previously existing condition and 
check for is_new instead?  I.e.:

			if ((!patch->is_delete && !patch->new_name) ||
			    (!patch->is_new    && !patch->old_name)) {

or

			if (!(patch->is_delete || patch->new_name) ||
			    !(patch->is_new    || patch->old_name)) {

René