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

From: Calvin Wan <calvinwan@google.com>
Date: Apr 22, 2024, 19:48 UTC
Message-ID: <20240422194824.340464-1-calvinwan@google.com>
In-Reply-To: <20200420224310.9989-1-carenas@gmail.com>

Carlo Marcelo Arenas Belón  <carenas@gmail.com> writes:

Show 28 quoted lines

> 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.