Subject: [DISCUSS] Possible error in libgcrypt.h

[DISCUSS] Possible error in libgcrypt.h

From: Tor Arntsen <tor_at_spacetec.no>
Date: Wed, 23 Jun 2010 11:21:36 +0200

Could someone have a look at the following, please? I could be wrong so some
other eyeballs may be able to spot it. Anyway, here goes:

In kex.c we have the following piece:

#if LIBSSH2_MD5
        {
            libssh2_md5_ctx fingerprint_ctx;

            libssh2_md5_init(&fingerprint_ctx);
            libssh2_md5_update(fingerprint_ctx, session->server_hostkey,
                               session->server_hostkey_len);
            libssh2_md5_final(fingerprint_ctx, session->server_hostkey_md5);
        }
#ifdef LIBSSH2DEBUG
.
Note the 'libssh2_md5_final(fingerprint_ctx, session->server_hostkey_md5);'
I find libssh2_md5_final as a #define in two places: openssl.h and libgcrypt.h
If I have understood it correctly libgcrypt.h would be used if libgcrypt is
used instead of openssl, right?

The define:
#define libssh2_md5_final(ctx, out) \
  memcpy (out, gcry_md_read (ctx, 0), 20), gcry_md_close (ctx)

So that translates to
memcpy (session->server_hostkey_md5, gcry_md_read (ctx, 0), 20);
However, server_hostkey_md5 is

unsigned char server_hostkey_md5[MD5_DIGEST_LENGTH]; (libssh2_priv.h)
and
#define MD5_DIGEST_LENGTH 16
is set in libgcrypt.h

so it looks like the memcpy will overflow session->server_hostkey_md5.
Is this a copy-paste error (as SHA_DIGEST_LENGTH is 20 and there is a similar memcpy define for libssh2_sha1_final, with 20), or am I overlooking something? Or is the following a fix?

-Tor
Received on 2010-06-23