Subject: About allocation/desallocation

About allocation/desallocation

From: Jean-Louis CHARTON <Jean-Louis.CHARTON_at_oikialog.com>
Date: Sat, 28 Mar 2009 20:15:56 +0100

Hi,

After Daniel remark "Why alloc for 7 bytes? Why alloc for 7 *fixed*
bytes?";

I reread userauth.c code for functions userauth_publickey_fromfile and
userauth_hostbased_fromfile and there is something that IMO is a bit
curious:

In userauth_hostbased_fromfile, we have (comments omitted):

        if (file_read_publickey(session, &session->userauth_host_method,
                                &session->userauth_host_method_len,
                                &pubkeydata, &pubkeydata_len,
                                publickey)) {
            return -1;
        }

        session->userauth_host_packet_len =
            username_len + session->userauth_host_method_len +
hostname_len +
            local_username_len + pubkeydata_len + 48;

        session->userauth_host_s = session->userauth_host_packet =
            LIBSSH2_ALLOC(session,
                          session->userauth_host_packet_len + 4 +
                          (4 + session->userauth_host_method_len) +
                          (4 + pubkeydata_len));
        if (!session->userauth_host_packet) {
            LIBSSH2_FREE(session, session->userauth_host_method);
            session->userauth_host_method = NULL;
            return -1;
        }
        ...

and a few lines below:
        if (libssh2_file_read_privatekey
            (session, &privkeyobj, &abstract,
session->userauth_host_method,
             session->userauth_host_method_len, privatekey, passphrase))
{
            LIBSSH2_FREE(session, session->userauth_host_method);
            session->userauth_host_method = NULL;
            LIBSSH2_FREE(session, session->userauth_host_packet);
            session->userauth_host_packet = NULL;
            return -1;
        }

=> Here, notice that on failure, session->userauth_host_method is freed.
Also notice that in userauth_hostbased_fromfile, there is no call to
free pubkeydata.

Now in userauth_publickey_fromfile we have (comments omitted):

        if (file_read_publickey(session, &session->userauth_pblc_method,
                                &session->userauth_pblc_method_len,
                                &pubkeydata, &pubkeydata_len,publickey))
{
            return -1;
        }

        session->userauth_pblc_packet_len =
            username_len + session->userauth_pblc_method_len +
pubkeydata_len +
            45;

        session->userauth_pblc_s = session->userauth_pblc_packet =
            LIBSSH2_ALLOC(session,
                          session->userauth_pblc_packet_len + 4 +
                          (4 + session->userauth_pblc_method_len)
                          + (4 + pubkeydata_len));
        if (!session->userauth_pblc_packet) {
            LIBSSH2_FREE(session, session->userauth_pblc_method);
            session->userauth_pblc_method = NULL;
            LIBSSH2_FREE(session, pubkeydata);
            return -1;
        }

and a few lines below :
        _libssh2_htonu32(session->userauth_pblc_s, pubkeydata_len);
        session->userauth_pblc_s += 4;
        memcpy(session->userauth_pblc_s, pubkeydata, pubkeydata_len);
        session->userauth_pblc_s += pubkeydata_len;
        LIBSSH2_FREE(session, pubkeydata);

=> Here, notice that on failure, BOTH session->userauth_pblc_method AND
pubkeydata are freed
and that there is also an explicit call to free pubkeydata.

So I believe there is a memory leak in userauth_hostbased_fromfile.

Am I right or is there something I've missed?

JL

------------------------------------------------------------------------------
_______________________________________________
libssh2-devel mailing list
libssh2-devel_at_lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libssh2-devel
Received on 2009-03-28