]> git.ipfire.org Git - thirdparty/rspamd.git/commitdiff
[Fix] fuzzy_storage: harden network input paths
authorVsevolod Stakhov <vsevolod@rspamd.com>
Mon, 18 May 2026 11:01:23 +0000 (12:01 +0100)
committerVsevolod Stakhov <vsevolod@rspamd.com>
Mon, 18 May 2026 11:01:23 +0000 (12:01 +0100)
Three defensive fixes for user-controlled input over UDP/TCP:

* accept_fuzzy_socket: reset msg_namelen back to the buffer capacity
  before every recvmsg/recvmmsg call. The kernel overwrites msg_namelen
  with the actual source address size on output; on the non-recvmmsg
  path the for(;;) loop reused the same msghdr across calls, so a
  larger source address (e.g. IPv6 after IPv4) was silently truncated
  by the kernel and the trailing bytes of the parsed sockaddr came
  from stale stack memory.

* rspamd_fuzzy_tcp_io: validate the reconstructed 16-bit frame length
  before folding it into cur_frame_state. The state machine only has
  14 bits for the length (top two bits are flags), so values with bit
  14 or 15 set were silently masked off, letting a client smuggle a
  large advertised size while the server parsed a much smaller frame.
  Now any length above FUZZY_TCP_BUFFER_LENGTH or equal to zero closes
  the connection immediately.

* rspamd_fuzzy_make_reply: clamp mf_result->n_extra_flags to
  RSPAMD_FUZZY_MAX_EXTRA_FLAGS before the memcpy into the fixed-size
  rep_v2->extra_flags[7]. All current backends already bound this
  value, but the frontend was trusting them; clamp defensively so a
  future backend bug cannot become an OOB write on the reply struct.

src/fuzzy_storage.c

index ece40aa2dda0b506ab7de3dc22960f555b0aff3a..b8a88039f5ff779f4614d07f041949c21379aa50 100644 (file)
@@ -717,9 +717,21 @@ rspamd_fuzzy_make_reply(struct rspamd_fuzzy_cmd *cmd,
 
                        /* Extra flags from multiflag result */
                        if (mf_result) {
-                               rep_v2->n_extra_flags = mf_result->n_extra_flags;
+                               uint8_t n_extra = mf_result->n_extra_flags;
+                               /*
+                                * rep_v2->extra_flags is fixed-size [RSPAMD_FUZZY_MAX_EXTRA_FLAGS].
+                                * All current backends bound n_extra_flags to this maximum, but
+                                * clamp defensively here so a future backend bug cannot turn
+                                * into an OOB write on the reply struct.
+                                */
+                               if (G_UNLIKELY(n_extra > RSPAMD_FUZZY_MAX_EXTRA_FLAGS)) {
+                                       msg_warn("backend returned n_extra_flags=%d > max %d, clamping",
+                                                        (int) n_extra, (int) RSPAMD_FUZZY_MAX_EXTRA_FLAGS);
+                                       n_extra = RSPAMD_FUZZY_MAX_EXTRA_FLAGS;
+                               }
+                               rep_v2->n_extra_flags = n_extra;
                                memcpy(rep_v2->extra_flags, mf_result->extra_flags,
-                                          sizeof(struct rspamd_fuzzy_flag_entry) * mf_result->n_extra_flags);
+                                          sizeof(struct rspamd_fuzzy_flag_entry) * n_extra);
                        }
 
                        if (flags & RSPAMD_FUZZY_REPLY_DELAY) {
@@ -2036,6 +2048,17 @@ accept_fuzzy_socket(EV_P_ ev_io *w, int revents)
        if (revents == EV_READ) {
                ev_now_update_if_cheap(ctx->event_loop);
                for (;;) {
+                       /*
+                        * The kernel overwrites msg_namelen on each recvmsg/recvmmsg call
+                        * with the actual size of the received source address. If we don't
+                        * reset it back to the buffer capacity before the next call, the
+                        * kernel will treat that smaller value as the buffer size and
+                        * truncate a larger incoming address (e.g. IPv6 after IPv4),
+                        * leaving stale stack bytes mixed into the parsed sockaddr.
+                        */
+                       for (int i = 0; i < MSGVEC_LEN; i++) {
+                               MSG_FIELD(msg[i], msg_namelen) = salen;
+                       }
 #ifdef HAVE_RECVMMSG
                        r = recvmmsg(w->fd, msg, MSGVEC_LEN, 0, NULL);
 #else
@@ -2235,7 +2258,22 @@ rspamd_fuzzy_tcp_io(EV_P_ ev_io *w, int revents)
                                        uint16_t first_byte = session->cur_frame_state & 0xFF;
                                        uint16_t second_byte = session->input_buf[processed_offset];
                                        /* Reconstruct in little-endian byte order */
-                                       session->cur_frame_state = 0xC000 | (first_byte | (second_byte << 8));
+                                       uint16_t real_len = first_byte | (second_byte << 8);
+                                       /*
+                                        * Length is stored in the low 14 bits of cur_frame_state
+                                        * (top two bits are state flags). FUZZY_TCP_BUFFER_LENGTH
+                                        * fits in 14 bits, so any value with bit 14 or 15 set is
+                                        * invalid and would otherwise be silently masked off,
+                                        * creating a protocol-smuggling primitive. Reject here.
+                                        */
+                                       if (real_len == 0 || real_len > FUZZY_TCP_BUFFER_LENGTH) {
+                                               msg_err("invalid frame length %d from %s, closing connection",
+                                                               (int) real_len,
+                                                               rspamd_inet_address_to_string(session->common.addr));
+                                               REF_RELEASE(session);
+                                               return;
+                                       }
+                                       session->cur_frame_state = 0xC000 | real_len;
                                        processed_offset++;
                                }
                                else {