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
>From 13238b88a38ecf2a24e7ee8c68947c9c620deb15 Mon Sep 17 00:00:00 2001
From: Tor Arntsen <[email protected]>
Date: Wed, 23 Jun 2010 10:57:50 +0200
Subject: [PATCH] Don't overflow MD5 server hostkey
---
src/libgcrypt.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/libgcrypt.h b/src/libgcrypt.h
index a5366f9..3a7a3fa 100644
--- a/src/libgcrypt.h
+++ b/src/libgcrypt.h
@@ -71,7 +71,7 @@
#define libssh2_md5_init(ctx) gcry_md_open (ctx, GCRY_MD_MD5, 0);
#define libssh2_md5_update(ctx, data, len) gcry_md_write (ctx, data, len)
#define libssh2_md5_final(ctx, out) \
- memcpy (out, gcry_md_read (ctx, 0), 20), gcry_md_close (ctx)
+ memcpy (out, gcry_md_read (ctx, 0), 16), gcry_md_close (ctx)
#define libssh2_md5(message, len, out) \
gcry_md_hash_buffer (GCRY_MD_MD5, out, message, len)
--
1.7.1
Alternatively, this one instead:
>From e2ec4c952fa37ffe832eb664f48d334c3d800085 Mon Sep 17 00:00:00 2001
From: Tor Arntsen <[email protected]>
Date: Wed, 23 Jun 2010 11:15:34 +0200
Subject: [PATCH] Don't overflow MD5 server hostkey
Use SHA_DIGEST_LENGTH and MD5_DIGEST_LENGTH
in memcpy instead of hardcoded values. An incorrect
value was used for md5.
---
src/libgcrypt.h | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/libgcrypt.h b/src/libgcrypt.h
index a5366f9..d925700 100644
--- a/src/libgcrypt.h
+++ b/src/libgcrypt.h
@@ -63,7 +63,7 @@
#define libssh2_sha1_init(ctx) gcry_md_open (ctx, GCRY_MD_SHA1, 0);
#define libssh2_sha1_update(ctx, data, len) gcry_md_write (ctx, data, len)
#define libssh2_sha1_final(ctx, out) \
- memcpy (out, gcry_md_read (ctx, 0), 20), gcry_md_close (ctx)
+ memcpy (out, gcry_md_read (ctx, 0), SHA_DIGEST_LENGTH), gcry_md_close (ctx)
#define libssh2_sha1(message, len, out) \
gcry_md_hash_buffer (GCRY_MD_SHA1, out, message, len)
@@ -71,7 +71,7 @@
#define libssh2_md5_init(ctx) gcry_md_open (ctx, GCRY_MD_MD5, 0);
#define libssh2_md5_update(ctx, data, len) gcry_md_write (ctx, data, len)
#define libssh2_md5_final(ctx, out) \
- memcpy (out, gcry_md_read (ctx, 0), 20), gcry_md_close (ctx)
+ memcpy (out, gcry_md_read (ctx, 0), MD5_DIGEST_LENGTH), gcry_md_close (ctx)
#define libssh2_md5(message, len, out) \
gcry_md_hash_buffer (GCRY_MD_MD5, out, message, len)
--
1.7.1
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
