simon-git: putty (master): Simon Tatham

[Commits to Tartarus hosted VCS]

TL;DR:
  108baae6 Add further missing delete_callbacks_for_context.
  f99aeb31 mainchan.c: rewrite handling of open-failure aborts.
  98ed37f5 ssh2connection: missing free of antispoof_prompt.
  128d001c SSH-1: disable trust sigils after session starts.
  07a43efb SSH-1: free mainchan_chan on destruction.
  65b3c93a Uppity: free the packet protocol layers on exit.

Repository:     https://git.tartarus.org/simon/putty.git
On the web:     https://git.tartarus.org/?p=simon/putty.git
Branch updated: master
Committer:      Simon Tatham <anakin at pobox.com>
Date:           2019-04-20 10:01:36

commit 108baae60ee33912abeaa95b6761bee4ad6bab82
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=108baae60ee33912abeaa95b6761bee4ad6bab82;hp=4dcc0fddf773174132d579e26b41cbda547b2bb5
Author: Simon Tatham <anakin at pobox.com>
Date:   Sat Apr 20 08:20:34 2019 +0100

    Add further missing delete_callbacks_for_context.
    
    Having explicitly _stated_ in commit 4dcc0fddf the principle that if
    you ever queue a toplevel callback on a freeable object then you
    should also call delete_callbacks_for_context on that object before
    freeing it, I realised I'd never actually gone through and checked
    methodically at every call site of queue_toplevel_callback. So I did,
    and naturally, I found several missing ones.

 scpserver.c        | 3 +++
 terminal.c         | 1 +
 windows/winhsock.c | 2 ++
 windows/winnet.c   | 1 +
 4 files changed, 7 insertions(+)

commit f99aeb312976ccefd62d84f3fc5e8a52943a36bc
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=f99aeb312976ccefd62d84f3fc5e8a52943a36bc;hp=108baae60ee33912abeaa95b6761bee4ad6bab82
Author: Simon Tatham <anakin at pobox.com>
Date:   Sat Apr 20 08:22:00 2019 +0100

    mainchan.c: rewrite handling of open-failure aborts.
    
    This is another case where a stale pointer bug could have arisen from
    a toplevel callback going off after an object was freed.
    
    But here, just adding delete_callbacks_for_context wouldn't help. The
    actual context parameter for the callback wasn't mainchan itself; it
    was a tiny separate object, allocated to hold just the parameters of
    the function the callback wanted to call. So if _those_ parameters
    became stale before the callback was triggered, then even
    delete_callbacks_for_context wouldn't have been able to help.
    
    Also, mainchan itself would have been freed moments after this
    callback was queued, so moving it to be a callback on mainchan itself
    wouldn't help.
    
    Solution: move the callback right out to Ssh, by introducing a new
    ssh_sw_abort_deferred() which is just like ssh_sw_abort but does its
    main work in a toplevel callback. Then ssh.c's existing call to
    delete_callbacks_for_context will clean it up if necessary.

 mainchan.c | 24 ++----------------------
 ssh.c      | 22 ++++++++++++++++++++++
 ssh.h      |  1 +
 3 files changed, 25 insertions(+), 22 deletions(-)

commit 98ed37f517eedae2077c67da736871861d0abddc
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=98ed37f517eedae2077c67da736871861d0abddc;hp=f99aeb312976ccefd62d84f3fc5e8a52943a36bc
Author: Simon Tatham <anakin at pobox.com>
Date:   Sat Apr 20 08:19:44 2019 +0100

    ssh2connection: missing free of antispoof_prompt.
    
    Just noticed that if the session were to abort while that variable
    contains some allocated data, it wouldn't be freed.

 ssh2connection.c | 4 ++++
 1 file changed, 4 insertions(+)

commit 128d001c3eebae15fe97fc18dc48d8939ae72e98
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=128d001c3eebae15fe97fc18dc48d8939ae72e98;hp=98ed37f517eedae2077c67da736871861d0abddc
Author: Simon Tatham <anakin at pobox.com>
Date:   Sat Apr 20 08:24:16 2019 +0100

    SSH-1: disable trust sigils after session starts.
    
    This exactly replicates the way it's done in SSH-2: at the start of
    the connection layer we set the trust status to untrusted, and if that
    reports that it didn't give any indication to the user, we fall back
    to presenting an interactive anti-spoofing prompt.
    
    I don't know how I forgot to do that in SSH-1, and even more, how we
    haven't noticed for a month. We noticed the same bug in _Rlogin_
    within a day of the 0.71 release, after all!

 ssh1connection-client.c |  5 +++++
 ssh1connection-server.c |  5 +++++
 ssh1connection.c        | 39 +++++++++++++++++++++++++++++++++++++++
 ssh1connection.h        |  5 +++++
 4 files changed, 54 insertions(+)

commit 07a43efb1f67eb0e7bac127d808d5836a9417ff9
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=07a43efb1f67eb0e7bac127d808d5836a9417ff9;hp=128d001c3eebae15fe97fc18dc48d8939ae72e98
Author: Simon Tatham <anakin at pobox.com>
Date:   Sat Apr 20 09:37:54 2019 +0100

    SSH-1: free mainchan_chan on destruction.
    
    We went through the tree234 of ordinary channels freeing the thing at
    the other end of each one, but of course in SSH-1 the main session
    channel is not stored in that tree due to its totally different shape
    in the protocol. So naturally I forgot to free _that_ one.

 ssh1connection.c | 2 ++
 1 file changed, 2 insertions(+)

commit 65b3c93a8ec0e916cbf57b9bc824dde5b6b535ef
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=65b3c93a8ec0e916cbf57b9bc824dde5b6b535ef;hp=07a43efb1f67eb0e7bac127d808d5836a9417ff9
Author: Simon Tatham <anakin at pobox.com>
Date:   Sat Apr 20 09:38:54 2019 +0100

    Uppity: free the packet protocol layers on exit.
    
    This bug and the one in the previous commit combined to mean that when
    an SSH-1 connection through Uppity is terminated, the pty backend for
    its main session channel was never cleaned up. (Firstly because
    ssh1_connection_free never got called, and secondly because that in
    turn forgot to free its mainchan.)
    
    The effect of that in turn was that a _subsequent_ connection to the
    same Uppity (using the new listening-socket mode) would likely reuse
    the same fd for its pty, and the insertions into the ptyfds tree in
    uxpty.c would silently fail because an existing Pty was already
    occupying them, leading to a segfault when that Pty in turn responded
    to events on a pty it didn't really own and tried to call back to a
    seat that didn't exist any more.

 sshserver.c | 2 ++
 1 file changed, 2 insertions(+)