From 1199398188c1a5ee0e8bc147400d8532bc874700 Mon Sep 17 00:00:00 2001 From: Andre Noll Date: Thu, 3 Mar 2011 15:09:53 +0100 Subject: [PATCH] RC4: Fix invalid read. Commit 7cb8fa26 (May 2010) created a target buffer for the RC4-encoded data which is slightly larger than the input buffer because openssl apparently wrote beyond the size it was told to write. As it turns out, this was not enough as RC4() may also read-overflow the input buffer. Valgrind says on Linux/x86_64: ==2423== Invalid read of size 8 ==2423== at 0x5312020: RC4 (in /lib/libcrypto.so.0.9.8) ==2423== by 0x40F01D: rc4_send_bin_buffer (crypt.c:224) ==2423== by 0x40C724: com_stat (command.c:391) ==2423== by 0x40BABF: handle_connect (command.c:838) ==2423== by 0x408330: command_post_select (server.c:404) ==2423== by 0x41B5DA: schedule (sched.c:76) ==2423== by 0x4089C3: main (server.c:581) ==2423== Address 0x6cefeb8 is 232 bytes inside a block of size 235 alloc'd ==2423== at 0x4C275A2: realloc (vg_replace_malloc.c:525) ==2423== by 0x40DE74: para_realloc (string.c:40) ==2423== by 0x40E324: make_message (string.c:134) ==2423== by 0x40C5D0: com_stat (command.c:328) ==2423== by 0x40BABF: handle_connect (command.c:838) ==2423== by 0x408330: command_post_select (server.c:404) ==2423== by 0x41B5DA: schedule (sched.c:76) ==2423== by 0x4089C3: main (server.c:581) Fix this by treating the last len % 8 bytes of the input separately. It's ugly but it does silence valgrind and should not be noticeably slower since we are only doing one extra copy of at most 7 bytes. We need to round the input size up and down to a multiple of 8, so this patch introduces generic macros in para.h for this purpose. --- crypt.c | 12 ++++++++++-- para.h | 10 ++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/crypt.c b/crypt.c index 917948c6..cdcb6149 100644 --- a/crypt.c +++ b/crypt.c @@ -203,6 +203,8 @@ int para_encrypt_buffer(RSA *rsa, unsigned char *inbuf, return ret < 0? -E_ENCRYPT : ret; } +#define RC4_ALIGN 8 + /** * Encrypt and send a buffer. * @@ -218,10 +220,16 @@ int rc4_send_bin_buffer(struct rc4_context *rc4c, const char *buf, size_t len) { int ret; unsigned char *tmp; + static unsigned char remainder[RC4_ALIGN]; + size_t l1 = ROUND_DOWN(len, RC4_ALIGN), l2 = ROUND_UP(len, RC4_ALIGN); assert(len); - tmp = para_malloc(len + 8); - RC4(&rc4c->send_key, len, (const unsigned char *)buf, tmp); + tmp = para_malloc(l2); + RC4(&rc4c->send_key, l1, (const unsigned char *)buf, tmp); + if (len > l1) { + memcpy(remainder, buf + l1, len - l1); + RC4(&rc4c->send_key, len - l1, remainder, tmp + l1); + } ret = write_all(rc4c->fd, (char *)tmp, &len); free(tmp); return ret; diff --git a/para.h b/para.h index c5e12fe6..f095a6aa 100644 --- a/para.h +++ b/para.h @@ -165,6 +165,16 @@ _static_inline_ long int para_random(unsigned max) return ((max + 0.0) * (random() / (RAND_MAX + 1.0))); } +/** Round up x to next multiple of y. */ +#define ROUND_UP(x, y) ({ \ + const typeof(y) _divisor = y; \ + ((x) + _divisor - 1) / _divisor * _divisor; }) + +/** Round down x to multiple of y. */ +#define ROUND_DOWN(x, y) ({ \ + const typeof(y) _divisor = y; \ + (x) / _divisor * _divisor; }) + /** Divide and round up to next integer. */ #define DIV_ROUND_UP(x, y) ({ \ typeof(y) _divisor = y; \ -- 2.39.5