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 917948c6185710fb828b441025b8d46a0fdc2924..cdcb6149d04254a0def3bcc903823f71ae3a89e5 100644 (file)
--- 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 c5e12fe6da3ef864bdc16bb0eaa178a8b5965cee..f095a6aa385d2f717048f1e25e15741a15632617 100644 (file)
--- 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; \