Jan Palus wrote on Wed, Jul 11, 2018 at 13:46:35 +0200:
> [[[
> Fix segfault when subversion is built with gnome keyring support and keyring
> password lookup fails.
>
> * subversion/libsvn_subr/simple_providers.c
> (svn_auth__simple_creds_cache_get): Initialize 'done' with FALSE since ie
> password_get_gnome_keyring sets 'done' only to TRUE. In case of error
> it leaves unintialized 'done' (usually non 0) and default_password with NULL
> causing segfault at subsequent strcmp.
Thanks for the patch; however, I don't think that's the correct fix.
The svn_auth__password_get_t requires implementations (= password_get_gnome_keyring())
to either set *DONE to something or return an error (of type svn_error_t*).
password_get_gnome_keyring() violates that API, so password_get_gnome_keyring()
should be fixed.
Does the following patch fix the problem? Speaking of which, could you explain
how to reproduce the problem?
Does anyone want to audit the other related functions (vertically —
gnome_keyring.c — and horizontally — other svn_auth__password_get_t
implementations) to see whether any of them is also affected?
[[[
Index: subversion/libsvn_auth_gnome_keyring/gnome_keyring.c
===================================================================
--- subversion/libsvn_auth_gnome_keyring/gnome_keyring.c (revision 1835745)
+++ subversion/libsvn_auth_gnome_keyring/gnome_keyring.c (working copy)
@@ -118,6 +118,8 @@ password_get_gnome_keyring(svn_boolean_t *done,
{
GError *gerror = NULL;
gchar *gpassword;
+
+ *done = FALSE;
if (!available_collection(non_interactive, pool))
return SVN_NO_ERROR;
@@ -129,6 +131,7 @@ password_get_gnome_keyring(svn_boolean_t *done,
NULL);
if (gerror)
{
+ /* ### TODO: return or log the error? */
g_error_free(gerror);
}
else if (gpassword)
]]]
Cheers,
Daniel
(I don't recall whether I build with gnome-keyring...)
Received on 2018-07-12 17:08:20 CEST