Re: ssh_set_callbacks bug ?

[Andreas Schneider <asn@xxxxxxxxxxxxxx>]

On Sunday 14 February 2016 11:21:10 Giovanni Panozzo wrote:
> Hi to all. I just subscribed to the list... but I think I have been
> subscribed in the past :)
> 
> While developing with libssh I have found the following problem:
> 
> The lines at the end of ssh_set_callbacks() are:
> 
> /* LEGACY */
> if (ssh_get_log_callback() == NULL && cb->log_function) {
>      ssh_set_log_callback(ssh_legacy_log_callback);
>      ssh_set_log_userdata(session);
> }
> 
> the last line store "session" (a short life variable) in a global variable.
> That pointer to "sesson" may become later no longer available during a
> program run. For example when you close/destroy your first session and
> then reopen a second and enable logging callback.
> You can try to setup again callbacks to the second session, but
>   if (ssh_get_log_callback() == NULL
> will prevent the call to ssh_set_log_userdata().
> 
> Crashing example here:
> 
> --------------
> 
> #include <stdio.h>
> #include <malloc.h>
> #include <libssh/libssh.h>
> #include <libssh/callbacks.h>
> #include <stdlib.h>
> 
> void mylogfunc(ssh_session session, int priority, const char *message,
> void *userdata)
> {
>          printf("LOG MESSAGE: userdata=%p message=%s\n", userdata,message);
> }
> 
> void onerun()
> {
> 
>          ssh_session sshs;
>          struct ssh_callbacks_struct *cb;
>          int rc;
>          int verbosity = SSH_LOG_PROTOCOL;
> 
>          sshs = ssh_new();
>          if (sshs == NULL) {
>                  printf("ssh_new() failed\n");
>                  return;
>          }
> 
>          ssh_options_set(sshs, SSH_OPTIONS_HOST, "localhost");
>          ssh_options_set(sshs, SSH_OPTIONS_LOG_VERBOSITY, &verbosity);
> 
>          cb = malloc(sizeof(struct ssh_callbacks_struct));
>          memset(cb, 0, sizeof(struct ssh_callbacks_struct));
>          ssh_callbacks_init(cb);
>          cb->log_function = mylogfunc;
>          cb->userdata = NULL;
>          ssh_set_callbacks(sshs, cb);
> 
>          rc = ssh_connect(sshs);
>          if (rc != SSH_OK) {
>                  printf("Unable to connect to server\n");
>          }
>          ssh_disconnect(sshs);
> 
>          ssh_free(sshs);
>          free(cb);
> 
> }
> 
> int main()
> {
>          onerun();
>          // Allocate a new block of memory so ssh_new() will not reuse
> the old memory,
>          // making the pointer saved in ssh_set_callbacks() with
> ssh_set_log_userdata()
>          // erroneously available again
>          malloc(2000);
>          onerun();
> }
> 
> 
> -----
> As a workaround in user code, ssh_set_log_userdata(sshs) can be called
> just after a successful call to ssh_new.

I would suggest to use the new API and all your problems will be fixed :)

-- 
Andreas Schneider                   GPG-ID: CC014E3D
www.cryptomilk.org                asn@xxxxxxxxxxxxxx