Skip to content

Commit

Permalink
Fix limit undefined behavior crash in CLUSTER SLOT-STATS (#709)
Browse files Browse the repository at this point in the history
We did not set a default value for limit, but it will be used
in addReplyOrderBy later, the undefined behavior may crash the
server since the value could be negative and crash will happen
in addReplyArrayLen.

An interesting reproducible example (limit reuses the value of -1):
```
> cluster slot-stats orderby key-count desc limit -1
(error) ERR Limit has to lie in between 1 and 16384 (maximum number of slots).
> cluster slot-stats orderby key-count desc
Error: Server closed the connection
```

Set the default value of limit to 16384.

---------

Signed-off-by: Binbin <[email protected]>
  • Loading branch information
enjoy-binbin authored Jun 28, 2024
1 parent 7f7ef9a commit 518f0bf
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/cluster_slot_stats.c
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ void clusterSlotStatsCommand(client *c) {
}
int i = 4; /* Next argument index, following ORDERBY */
int limit_counter = 0, asc_desc_counter = 0;
long limit;
long limit = CLUSTER_SLOTS;
while (i < c->argc) {
int moreargs = c->argc > i + 1;
if (!strcasecmp(c->argv[i]->ptr, "limit") && moreargs) {
Expand Down
3 changes: 2 additions & 1 deletion tests/unit/cluster/slot-stats.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ start_cluster 1 0 {tags {external:skip cluster}} {
# -----------------------------------------------------------------------------

start_cluster 1 0 {tags {external:skip cluster}} {

# SET keys for target hashslots, to encourage ordering.
set hash_tags [list 0 1 2 3 4]
set num_keys 1
Expand All @@ -220,6 +220,7 @@ start_cluster 1 0 {tags {external:skip cluster}} {

test "CLUSTER SLOT-STATS ORDERBY DESC correct ordering" {
set orderby "key-count"
assert_error "ERR*" {R 0 CLUSTER SLOT-STATS ORDERBY $orderby DESC LIMIT -1}
set slot_stats [R 0 CLUSTER SLOT-STATS ORDERBY $orderby DESC]
assert_slot_stats_monotonic_descent $slot_stats $orderby
}
Expand Down

0 comments on commit 518f0bf

Please sign in to comment.