BUG/MINOR: peers: reject message lengths above 32 bits before truncation

peer_recv_msg() decodes the stick-table message length with intdecode(),
which returns a 64-bit value, and stores it into a uint32_t before
checking it against the buffer size. The length is encoded on at most 5
bytes here, so it can reach 4328786159 (2^32 + 33818863). Such values
wrap to 0..33818863 when truncated, pass the size check, and desync the
parsing of the stream instead of being rejected.

Decode into a 64-bit local variable and compare it to trash.size before
storing it, so oversized lengths are now reported as ERRSIZE.

This only triggers on a malformed length sent by a configured peer,
i.e. a buggy or malicious one: a compliant peer never encodes such a
value, and peer sessions are expected to run on trusted networks. The
impact is limited to garbling that session; there is no buffer write
overflow, as only the truncated length is ever used to read data.

This has been there for a long time and can be backported to all stable.

diff --git a/src/peers.c b/src/peers.c
index 3282463c3539ce157e21fff12a9bfcbd247ada46..2973f2104856bbbb75b17198f682695ec50982dd 100644 (file)
--- a/src/peers.c
+++ b/src/peers.c
@@ -2566,6 +2566,7 @@ static inline int peer_treat_definemsg(struct appctx *appctx, struct peer *p,
 static inline int peer_recv_msg(struct appctx *appctx, char *msg_head, size_t msg_head_sz,
                                 uint32_t *msg_len, int *totl)
 {
+       uint64_t len;
        int reql;
        char *cur;
 
@@ -2594,7 +2595,7 @@ static inline int peer_recv_msg(struct appctx *appctx, char *msg_head, size_t ms
                goto incomplete;
 
        cur = msg_head;
-       *msg_len = intdecode(&cur, cur + reql);
+       len = intdecode(&cur, cur + reql);
        if (!cur) {
                /* the number is truncated, did we read enough ? */
                if (reql < msg_head_sz)
@@ -2607,15 +2608,17 @@ static inline int peer_recv_msg(struct appctx *appctx, char *msg_head, size_t ms
        }
        *totl += cur - msg_head;
 
+       /* the length must be checked before being truncated to 32 bits */
+       if (len > trash.size) {
+               /* Status code is not success, abort */
+               appctx->st0 = PEER_SESS_ST_ERRSIZE;
+               TRACE_PROTO("malformed message: too large length encoding", PEERS_EV_SESS_IO|PEERS_EV_RX_MSG|PEERS_EV_PROTO_ERR, appctx);
+               return -1;
+       }
+       *msg_len = len;
+
        /* Read message content */
        if (*msg_len) {
-               if (*msg_len > trash.size) {
-                       /* Status code is not success, abort */
-                       appctx->st0 = PEER_SESS_ST_ERRSIZE;
-                       TRACE_PROTO("malformed message: too large length encoding", PEERS_EV_SESS_IO|PEERS_EV_RX_MSG|PEERS_EV_PROTO_ERR, appctx);
-                       return -1;
-               }
-
                reql = applet_getblk(appctx, trash.area, *msg_len, *totl);
                if (reql <= 0) /* closed */
                        goto incomplete;