Page MenuHomePhabricator

setting knownhost option to NULL crashes, at least before connecting
Closed, ResolvedPublic


Calling ssh_options_set (session, SSH_OPTIONS_KNOWNHOSTS, NULL) should reset the option back to the default, however when calling this before connecting, it crashes:

#0 0x00007ffff78f47c1 in __strlen_avx2 () from /lib64/
#1 0x00007ffff781fc7e in strdup () from /lib64/
#2 0x00007ffff7b7c7e1 in ssh_path_expand_escape () from /lib64/
#3 0x00007ffff7b7db02 in ssh_options_set () from /lib64/
#4 0x00000000004006a5 in main ()

Because strdup(session->opts.sshdir); gets called when opts.sshdir is still NULL it fails. When calling ssh_options_set (session, SSH_OPTIONS_SSH_DIR, NULL); before setting the knownhosts file to the defaults it works fine because opts.sshdir is now the default value.

Event Timeline

asn added a subscriber: asn.

Jabuk, is that working with the new config parsting stuff?

No, this issue is still present it I am right.

AFAIK, setting this option to NULL should set also the underlying value opts.knownhosts to NULL and let the correct location resolve in the ssh_options_apply(), where we know the sshdir is already set.

options.c:610 in current master:

    v = value;
    if (v == NULL) {
        session->opts.knownhosts = ssh_path_expand_escape(session,

ssh_path_expand_escape is called immediately when calling ssh_options_set with SSH_OPTIONS_KNOWNHOSTS, which assumes opts.sshdir is set.

I guess this ssh_path_expand_escape call can be moved to ssh_options_apply()?

Yes, I think that would be the correct fix.

Should be resolved by the attached commits in both master and stable 0.8 branches.

This comment was removed by asn.