From: Andre Noll <maan@systemlinux.org>
Date: Thu, 3 Mar 2011 14:09:53 +0000 (+0100)
Subject: RC4: Fix invalid read.
X-Git-Tag: v0.4.6~21^2~2
X-Git-Url: http://git.tuebingen.mpg.de/?p=paraslash.git;a=commitdiff_plain;h=1199398188c1a5ee0e8bc147400d8532bc874700;ds=sidebyside

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.
---

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; \