Re: [PATCH] osxkeychain: restrict queries to requests with a valid host

[Calvin Wan <calvinwan@google.com>]

Carlo Marcelo Arenas Belón  <carenas@gmail.com> writes:
> make sure that requests to this helper to get credentials return early if
> there is no host ord the host is empty.
> 
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
>  contrib/credential/osxkeychain/git-credential-osxkeychain.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/contrib/credential/osxkeychain/git-credential-osxkeychain.c b/contrib/credential/osxkeychain/git-credential-osxkeychain.c
> index bcd3f575a3..2264a88c41 100644
> --- a/contrib/credential/osxkeychain/git-credential-osxkeychain.c
> +++ b/contrib/credential/osxkeychain/git-credential-osxkeychain.c
> @@ -69,6 +69,12 @@ static void find_internet_password(void)
>  	UInt32 len;
>  	SecKeychainItemRef item;
>  
> +	/*
> +	 * Require at valid host to fix CVE-2020-11008
> +	 */
> +	if (!host || !*host)
> +		return;
> +
>  	if (SecKeychainFindInternetPassword(KEYCHAIN_ARGS, &len, &buf, &item))
>  		return;
>  
> -- 
> 2.26.2.111.gbff22aa583
> 

We're currently using this patch downstream (removed the check for
!*host after updates to this file), but I was wondering whether this
change should also be in main. It seems like the discussion around this
stalled and there was no definitive conclusion, but the change also at
worst does nothing and could possibly be useful -- I see other functions
where we're checking for the existence of "host". I wasn't around when
all the changes around this CVE were happening so I'm not exactly sure
how useful this patch this and whether we can get rid of it or not.