Question about sshbuf

[Damien Miller]

On Mon, 23 May 2022, Ron Frederick wrote:

> >>        ...snip...
> >> out:
> >>        sshbuf_free(server_host_key_blob);
> >> 
> >> I believe the problem here is that the call to sshpkt_getb_froms() is
> >> returning an sshbuf in server_host_key_blob which is a reference to
> >> the string being consumed from the packet being read, setting that
> >> original packet as its parent. As a result, the “ssh” buffer now has
> >> a refcount of 2, and when it returns to the top of the do {...} while
> >> and tries to read another packet into “ssh”, it gets the error about
> >> the sshbuf being “read-only” (for good reason).
> > 
> > IMO the best fix for this would be to put a sshbuf_free() at the
> > start of the do {} loop (it will handle a NULL argument fine).
> 
> 
> Thanks Damien for the quick response.
> 
> Given that “ssh” is actually a “struct ssh *” and not a “struct sshbuf *”, what would be the right argument to pass into sshbuf_free()? It looks like it would need to be something like ssh->state->incoming_packet, but this calling code probably shouldn’t be poking into the internals like that. I don’t see a wrapper in packet.c which would do a free of that internal buffer.

I'd do this:

> >>                        /* If we've sent them data, they should reply */
> >>                        do {

                                    sshbuf_free(server_host_key_blob);
                                    server_host_key_blob = NULL;

> >>                                type = ssh_packet_read(ssh);
> >>                                if (type == SSH2_MSG_KEXGSS_HOSTKEY) {
> >>                                        debug("Received KEXGSS_HOSTKEY");
> >>                                        if (server_host_key_blob)
> >>                                                fatal("Server host key received more than once");
> >>                                        if ((r = sshpkt_getb_froms(ssh, &server_host_key_blob)) != 0)
> >>                                                fatal("Failed to read server host key: %s", ssh_err(r));
> >>                                }
> >>                        } while (type == SSH2_MSG_KEXGSS_HOSTKEY);