BUG/MINOR: peers: fix OOB heap write in dictionary cache update

When a peer sends a dictionary entry update with a value (the else
branch at line 2109), the entry id decoded from the wire was never
validated against dc->max_entries before being used as an array index
into dc->rx[].

A malicious peer can send id=N where N > 128 (PEER_STKT_CACHE_MAX_ENTRIES)
to:
  - dc->rx[id-1].de at line 2123: OOB read followed by atomic decrement
    and potential free of an attacker-controlled pointer via
    dict_entry_unref()
  - dc->rx[id-1].de = de at line 2124: OOB write of a heap pointer at
    an attacker-controlled offset (16-byte stride, ~64 GiB range)

The bounds check was added to the key-only branch in commit f9e51beec
("BUG/MINOR: peers: Do not ignore a protocol error for dictionary
entries.") but was never added to the with-value branch. The bug has
been present since dictionary support was introduced in commit
8d78fa7def5c ("MINOR: peers: Make peers protocol support new
"server_name" data type.").

Reachable from any TCP client that knows the configured peer name
(no cryptographic authentication on the peers protocol). Requires a
stick-table with "store server_key" in the configuration.

Fix by hoisting the bounds check above the branch so it covers both
paths.

Must be backported as far as 2.6.

diff --git a/src/peers.c b/src/peers.c
index 3e15710a888404b8745e75bae172832ae7ffb3fc..2968097aa0bf3d37f14835a74a50ab59842d01ee 100644 (file)
--- a/src/peers.c
+++ b/src/peers.c
@@ -2097,12 +2097,12 @@ int peer_treat_updatemsg(struct appctx *appctx, struct peer *p, int updt, int ex
                        }
 
                        dc = p->dcache;
+                       if (id > dc->max_entries) {
+                               TRACE_ERROR("malformed update message: invalid dict value", PEERS_EV_SESS_IO|PEERS_EV_PROTO_ERR, appctx, p, st);
+                               goto malformed_unlock;
+                       }
                        if (*msg_cur == end) {
                                /* Dictionary entry key without value. */
-                               if (id > dc->max_entries) {
-                                       TRACE_ERROR("malformed update message: invalid dict value", PEERS_EV_SESS_IO|PEERS_EV_PROTO_ERR, appctx, p, st);
-                                       goto malformed_unlock;
-                               }
                                /* IDs sent over the network are numbered from 1. */
                                de = dc->rx[id - 1].de;
                        }