[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] rpc: fix handling of SSH auth failure code



On Fri, Aug 31, 2018 at 04:16:45PM +0200, Andrea Bolognani wrote:
> On Fri, 2018-08-31 at 11:15 +0100, Daniel P. Berrangé wrote:
> > The result of libssh2_userauth_password is being assigned to 'ret' in
> > one branch and 'rc' in the other branch. Checks are all done against the
> > 'ret' variable, so one branch never does the correct check.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange redhat com>
> > ---
> >  src/rpc/virnetsshsession.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> The fix itself is correct, so:
> 
>   Reviewed-by: Andrea Bolognani <abologna redhat com>
> 
> However, see below.
> 
> >      /* determine exist status */
> > -    if (ret == LIBSSH2_ERROR_AUTHENTICATION_FAILED)
> > +    if (rc == LIBSSH2_ERROR_AUTHENTICATION_FAILED)
> >          return 1;
> >      else
> >          return -1;
> 
> Since we have a cleanup label right after this, it would make
> sense to change both 'return' to 'ret = ': that would ensure
> the function has a single return and also prevent 'password'
> from leaking when an error which is not AUTHENTICATION_FAILED
> is returned by libssh2_userauth_password().

Yes, makes sense.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]