In Redis 7.0 BZMPOP was introduced allowing to block for any of the provided sets to have at least one element. However this command introduced a change in command arguments for which the current generic blocking code for zset commands is not considering.

When issuing a bzmpop with timeout and count the command also blocks on the timeout/numkeys/max/min which are not keys. since the keys might not exist at the time the command is issued it may block on these keys. In case the user will add zset key which matches one of these arguments, it will cause the command to be unblocked and provide the results for this key. This can also be a potential security issue, since in case the user blocking on the command is not authorized for this new key, it would still get access to view the data.

To reproduce Client 1:

ACL SETUSER ranshid on +@all ~my* nopass
auth ranshid nopass
bzmpop 100 1 myzset max count 10 <--- blocks here

Client 2: zadd max 1 one 1 two 3 three

client 1 will be unblocked with this output:

1) "max"
2) 1) 1) "three"
      2) "3"
   2) 1) "two"
      2) "2"
   3) 1) "one"
      2) "1"
(6.81s)

Expected behavior Client 1 should have been blocked waiting for myzset to be written.

Additional information In my opinion for the blocking infrastructure we can use the same way we take keys for ACK using getKeysFromCommandWithSpecs in order to iterate and decide if to block on any of the keys.

Comment From: enjoy-binbin

sorry, my bad, this line should be

-    blockForKeys(c,BLOCKED_ZSET,c->argv+1,c->argc-2,count,timeout,NULL,&pos,NULL);
+    blockForKeys(c,BLOCKED_ZSET,keys,numkeys,count,timeout,NULL,&pos,NULL);

it will result like:

# take 100 (timeout) as a key
127.0.0.1:6379> bzmpop 100 1 myzset min count 1
1) "100"
2) 1) 1) "a"
      2) "1"
(23.91s)

# take 1 (numkeys) as a key
127.0.0.1:6379> bzmpop 100 1 myzset min count 1
1) "1"
2) 1) 1) "a"
      2) "1"
(2.72s)

# this one is ok
127.0.0.1:6379> bzmpop 100 1 myzset min count 1
1) "myzset"
2) 1) 1) "a"
      2) "1"
(3.45s)

# take min (min | max) as a key
127.0.0.1:6379> bzmpop 100 1 myzset min count 1
1) "min"
2) 1) 1) "a"
      2) "1"
(3.61s)

# take count (count) as a key
127.0.0.1:6379> bzmpop 100 1 myzset min count 1
1) "count"
2) 1) 1) "a"
      2) "1"
(3.56s)

i will fix this one, and leave the acl one for further discussion

Comment From: ranshid

sorry, my bad, this line should be

diff - blockForKeys(c,BLOCKED_ZSET,c->argv+1,c->argc-2,count,timeout,NULL,&pos,NULL); + blockForKeys(c,BLOCKED_ZSET,keys,numkeys,count,timeout,NULL,&pos,NULL);

it will result like:

```

take 100 (timeout) as a key

127.0.0.1:6379> bzmpop 100 1 myzset min count 1 1) "100" 2) 1) 1) "a" 2) "1" (23.91s)

take 1 (numkeys) as a key

127.0.0.1:6379> bzmpop 100 1 myzset min count 1 1) "1" 2) 1) 1) "a" 2) "1" (2.72s)

this one is ok

127.0.0.1:6379> bzmpop 100 1 myzset min count 1 1) "myzset" 2) 1) 1) "a" 2) "1" (3.45s)

take min (min | max) as a key

127.0.0.1:6379> bzmpop 100 1 myzset min count 1 1) "min" 2) 1) 1) "a" 2) "1" (3.61s)

take count (count) as a key

127.0.0.1:6379> bzmpop 100 1 myzset min count 1 1) "count" 2) 1) 1) "a" 2) "1" (3.56s) ```

i will fix this one, and leave the acl one for further discussion

Thank you for the fast response (I was going to fix it myself :) ) I do not think the ACL is still an issue after this fix is done.

Comment From: enjoy-binbin

Thank you for the fast response (I was going to fix it myself :) )

i am happen to see it (take me a while to take care of the test part), BZMPOP is kind caught my eye... thanks for the report, i must "did" a copy-paste error back then (without thinking it)

I do not think the ACL is still an issue after this fix is done.

ooh that would be great, i did not look into the acl part, the bug dose not require acl