[pacman-dev] [PATCH] Fix CVE-2016-5434 (DoS/loop and out of boundary read)

[Allan McRae]

On 28/09/17 22:02, Nils Freydank wrote:
> @@ -1057,9 +1079,21 @@ int SYMEXPORT alpm_extract_keyid(alpm_handle_t *handle, const char *identifier,
>  			return -1;
>  		}
>  
> +		if(length_check(len, pos, 4, handle, identifier)) {
> +			return -1;
> +		}
>  		pos = pos + 4;
>  
> +		/* pos got changed above, so an explicit check is necessary
> +		 * check for 2 as that catches another some lines down */
> +		if(length_check(len, pos, 2, handle, identifier)) {
> +			return -1;
> +		}
>  		hlen = (sig[pos] << 8) | sig[pos + 1];
> +

Why is there a double check here?  Sure pos got increased, but there is
not need to check that.  Only the second check before the read is needed.

Or I am missing something completely?

A