mirror of
https://github.com/hanzoai/redis.git
synced 2026-10-30 18:25:12 +00:00
76e35bb6e0
## Summary
Add overflow checks for attacker-controlled `uint32_t` length fields in
`clusterProcessPacket()` before adding them to `explen` (`uint32_t`).
Without these checks, crafted PUBLISH/PUBLISHSHARD and MODULE cluster
bus messages can wrap `explen` to a small value, bypassing the `totlen
!= explen` validation and causing heap out-of-bounds reads or OOM aborts
in the processing path.
## Problem
In `clusterProcessPacket()`, the expected packet length for PUBLISH and
MODULE messages is computed by summing struct overhead with
variable-length fields read from the packet header via `ntohl()`:
```c
// PUBLISH (line 2841-2846)
explen += sizeof(clusterMsgDataPublish) - 8 +
ntohl(hdr->data.publish.msg.channel_len) +
ntohl(hdr->data.publish.msg.message_len);
// MODULE (line 2855-2858)
explen += sizeof(clusterMsgModule) - 3 + ntohl(hdr->data.module.msg.len);
```
Both `channel_len + message_len` (PUBLISH) and `len` (MODULE) are
`uint32_t` values from the network packet. Their addition to `explen`
can overflow `uint32_t`, wrapping to a small value that matches
`totlen`, passing the size validation at line 2864.
**Example (PUBLISH):** With `channel_len = 0x80000000` and `message_len
= 0x80000000`, their sum overflows to `0`, making `explen` equal to just
the base struct size. An attacker sets `totlen` to match. The validation
passes, and the processing path at line 3273 calls
`createStringObject()` with the original 2GB lengths, reading far past
the received buffer.
**Attack vector:** Reachable via the cluster bus port (default: data
port + 10000), which does not require authentication by default.
## Fix
Extract `ntohl` values into local variables and check each addition
against `UINT32_MAX` before performing it. Reject the packet with a
warning log if overflow is detected. The computed `explen` is identical
to the original for all non-overflowing (legitimate) inputs.
---------
Signed-off-by: Sebastien Tardif <[email protected]>
Co-authored-by: debing.sun <[email protected]>
91 lines
3.2 KiB
Tcl
91 lines
3.2 KiB
Tcl
start_cluster 2 0 {tags {external:skip cluster}} {
|
|
|
|
test "PUBLISH with channel_len + message_len overflow is rejected" {
|
|
set CLUSTERMSG_TYPE_PUBLISH 4
|
|
set CLUSTERMSG_MIN_LEN 2256
|
|
|
|
set target_host [srv 0 host]
|
|
set target_bus_port [expr {[srv 0 port] + 10000}]
|
|
set node_id [R 1 CLUSTER MYID]
|
|
set sender_port [srv -1 port]
|
|
set sender_cport [expr {$sender_port + 10000}]
|
|
|
|
# sizeof(clusterMsgDataPublish) - 8 = 16 - 8 = 8
|
|
# With channel_len=0x80000000 and message_len=0x80000000, their sum
|
|
# overflows uint32 to 0, making explen = 2256 + 8 = 2264 without the fix.
|
|
# The fix detects the overflow before computing explen.
|
|
set channel_len 0x80000000
|
|
set message_len 0x80000000
|
|
set totlen [expr {$CLUSTERMSG_MIN_LEN + 8}] ;# 2264
|
|
|
|
set packet [build_cluster_bus_header $node_id $sender_port $sender_cport \
|
|
$CLUSTERMSG_TYPE_PUBLISH $totlen]
|
|
append packet [binary format I $channel_len]
|
|
append packet [binary format I $message_len]
|
|
|
|
if {$::tls} {
|
|
set fd [::tls::socket \
|
|
-cafile "$::tlsdir/ca.crt" \
|
|
-certfile "$::tlsdir/client.crt" \
|
|
-keyfile "$::tlsdir/client.key" \
|
|
$target_host $target_bus_port]
|
|
} else {
|
|
set fd [socket $target_host $target_bus_port]
|
|
}
|
|
fconfigure $fd -translation binary -buffering full
|
|
puts -nonewline $fd $packet
|
|
flush $fd
|
|
|
|
# The fix rejects the packet and logs a warning about the overflow.
|
|
wait_for_log_messages 0 {"*publish*overflow in length fields*"} 0 20 500
|
|
close $fd
|
|
|
|
# Verify the server is still responsive after the malformed packet.
|
|
assert_equal [R 0 PING] {PONG}
|
|
}
|
|
|
|
test "MODULE with len overflow is rejected" {
|
|
set CLUSTERMSG_TYPE_MODULE 9
|
|
set CLUSTERMSG_MIN_LEN 2256
|
|
|
|
set target_host [srv 0 host]
|
|
set target_bus_port [expr {[srv 0 port] + 10000}]
|
|
set node_id [R 1 CLUSTER MYID]
|
|
set sender_port [srv -1 port]
|
|
set sender_cport [expr {$sender_port + 10000}]
|
|
|
|
# sizeof(clusterMsgModule) - 3 = 16 - 3 = 13
|
|
# With module_len=0xFFFFFFFF, adding to base explen (2256+13=2269)
|
|
# overflows uint32, wrapping to 2268 without the fix.
|
|
# The fix detects the overflow before computing explen.
|
|
set module_len 0xFFFFFFFF
|
|
set totlen [expr {($CLUSTERMSG_MIN_LEN + 13 + $module_len) & 0xFFFFFFFF}] ;# 2268
|
|
|
|
set packet [build_cluster_bus_header $node_id $sender_port $sender_cport \
|
|
$CLUSTERMSG_TYPE_MODULE $totlen]
|
|
append packet [binary format W 0] ;# module_id
|
|
append packet [binary format I $module_len] ;# len
|
|
|
|
if {$::tls} {
|
|
set fd [::tls::socket \
|
|
-cafile "$::tlsdir/ca.crt" \
|
|
-certfile "$::tlsdir/client.crt" \
|
|
-keyfile "$::tlsdir/client.key" \
|
|
$target_host $target_bus_port]
|
|
} else {
|
|
set fd [socket $target_host $target_bus_port]
|
|
}
|
|
fconfigure $fd -translation binary -buffering full
|
|
puts -nonewline $fd $packet
|
|
flush $fd
|
|
|
|
# The fix rejects the packet and logs a warning about the overflow.
|
|
wait_for_log_messages 0 {"*module*overflow in length field*"} 0 20 500
|
|
close $fd
|
|
|
|
# Verify the server is still responsive after the malformed packet.
|
|
assert_equal [R 0 PING] {PONG}
|
|
}
|
|
|
|
}
|