BUG/MEDIUM: peers: check the available room before encoding dict values

peer_prepare_updatemsg() encodes stick-table entries into an update
message of <size> bytes but never checks that the encoded data actually
fits (the function still carries its "TODO: check size" comment). For
entries holding a server_key, the dictionary value, up to ~16 KB, is
copied unconditionally:

    /* Encode the length of the dictionary entry data */
    value_len = de->len;
    intencode(value_len, &end);
    /* Copy the data */
    memcpy(end, de->value.key, value_len);

The peers protocol is plaintext and unauthenticated, so a remote peer
can plant an entry whose server_key is larger than the room left in the
buffer. When the victim later teaches that entry, the memcpy() above
writes past the end of the 16384-byte trash buffer; this was confirmed
under ASan as a heap-buffer-overflow WRITE of size 16360, and it fires
again on every teach retry at the same address.

Let's verify that the value fits in the buffer before copying it, and
return 0 ("unable to encode the message") when it does not, which is
how the function already reports its other encoding failures.

This must be backported to all supported versions (server_key in
stick-tables dates back to 2.4).

diff --git a/src/peers.c b/src/peers.c
index 07b1097ccf759ea98dbdf8875d097a5219b4f1ac..4dabda33626167d44caeb97cf977ab78014b7a5e 100644 (file)
--- a/src/peers.c
+++ b/src/peers.c
@@ -887,6 +887,11 @@ int peer_prepare_updatemsg(char *msg, size_t size, struct peer_prep_params *p)
                                                /* Encode the length of the dictionary entry data */
                                                value_len = de->len;
                                                intencode(value_len, &end);
+                                               /* Make sure the value fits in the buffer */
+                                               if ((size_t)(end - msg) + value_len > size) {
+                                                       HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+                                                       return 0;
+                                               }
                                                /* Copy the data */
                                                memcpy(end, de->value.key, value_len);
                                                end += value_len;