Do the RC4_ALIGN dance also for sideband connections.
authorAndre Noll <maan@systemlinux.org>
Mon, 12 Nov 2012 14:15:43 +0000 (15:15 +0100)
committerAndre Noll <maan@systemlinux.org>
Sun, 2 Dec 2012 21:29:45 +0000 (22:29 +0100)
In commit 1199398 (March 2011) sc_send_bin_buffer() and
sc_recv_bin_buffer() received a fix for invalid reads/writes in
RC4(). The same fix is needed also for sc_crypt(), which was introduced
later in 2830b9f8 as part of the sideband API.

Without the fix, valgrind complains like this:

==14435== Invalid read of size 8
==14435==    at 0x510A03A: RC4 (in /ebio/maan/arch/Linux_x86_64/lib/libcrypto.so.0.9.8)
==14435==    by 0x40937E: sc_crypt (crypt.c:333)
==14435==    by 0x407233: sb_received (sideband.c:249)
==14435==    by 0x40770F: recv_sb (client_common.c:215)
==14435==    by 0x407FF3: client_post_select (client_common.c:427)
==14435==    by 0x406BE8: schedule (sched.c:70)
==14435==    by 0x404AE7: execute_client_command (client.c:115)
==14435==    by 0x404B77: extract_matches_from_command (client.c:133)
==14435==    by 0x404C16: generic_blob_complete (client.c:163)
==14435==    by 0x404E13: complete_lsblob (client.c:181)
==14435==    by 0x409AB0: create_matches (interactive.c:100)
==14435==    by 0x409BD0: completion_generator (interactive.c:134)
==14435==  Address 0x608bfd0 is 0 bytes inside a block of size 6 alloc'd
==14435==    at 0x4C274A8: malloc (vg_replace_malloc.c:236)
==14435==    by 0x405E64: para_malloc (string.c:65)
==14435==    by 0x40935E: sc_crypt (crypt.c:330)
==14435==    by 0x407233: sb_received (sideband.c:249)
==14435==    by 0x40770F: recv_sb (client_common.c:215)
==14435==    by 0x407FF3: client_post_select (client_common.c:427)
==14435==    by 0x406BE8: schedule (sched.c:70)
==14435==    by 0x404AE7: execute_client_command (client.c:115)
==14435==    by 0x404B77: extract_matches_from_command (client.c:133)
==14435==    by 0x404C16: generic_blob_complete (client.c:163)
==14435==    by 0x404E13: complete_lsblob (client.c:181)
==14435==    by 0x409AB0: create_matches (interactive.c:100)

See also 54cf5827.

crypt.c

diff --git a/crypt.c b/crypt.c
index b754c09..17026cc 100644 (file)
--- a/crypt.c
+++ b/crypt.c
@@ -319,19 +319,26 @@ int sc_recv_bin_buffer(struct stream_cipher_context *scc, char *buf,
 
 void sc_crypt(struct stream_cipher *sc, struct iovec *src, struct iovec *dst)
 {
+       size_t len = src->iov_len, l1, l2;
        RC4_KEY *key = &sc->key;
 
+       assert(len > 0);
+       assert(len < ((typeof(src->iov_len))-1) / 2);
+       l1 = ROUND_DOWN(len, RC4_ALIGN);
+       l2 = ROUND_UP(len, RC4_ALIGN);
+
        *dst = (typeof(*dst)) {
-               /*
-                * Add one for the terminating zero byte. Integer overflow is
-                * no problem here as para_malloc() aborts when given a zero
-                * size argument.
-                */
-               .iov_base = para_malloc(src->iov_len + 1),
-               .iov_len = src->iov_len
+               /* Add one for the terminating zero byte. */
+               .iov_base = para_malloc(l2 + 1),
+               .iov_len = len
        };
-       RC4(key, src->iov_len, src->iov_base, dst->iov_base);
-       ((char *)dst->iov_base)[dst->iov_len] = '\0';
+       RC4(key, l1, src->iov_base, dst->iov_base);
+       if (len > l1) {
+               unsigned char remainder[RC4_ALIGN] = "";
+               memcpy(remainder, src->iov_base + l1, len - l1);
+               RC4(key, len - l1, remainder, dst->iov_base + l1);
+       }
+       ((char *)dst->iov_base)[len] = '\0';
 }
 
 void hash_function(const char *data, unsigned long len, unsigned char *hash)