[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, 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().

-- 
Andrea Bolognani / Red Hat / Virtualization


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