Redis [BUG] A possible divide by zero bug in redis-cli.c

In redis-cli.c, the function clusterManagerNodeMasterRandom has the following code:

int master_count = 0;
listIter li;
listNode *ln;
listRewind(cluster_manager.nodes, &li);
while ((ln = listNext(&li)) != NULL) {
    clusterManagerNode *n = ln->value;
    if (n->flags & CLUSTER_MANAGER_FLAG_SLAVE) continue;
    master_count++;
}

srand(time(NULL));
idx = rand() % master_count;

master_count is used as a divisor and it may be zero if it is never increased in the loop, leading to a potential divide by zero bug.

Comment From: oranagra

a cluster with no masters? sounds like a cluster with no cluster.

do you have reason to believe this is reachable? (even if that's true, seems like invalid use of the cli).

Comment From: yiyuaner

a cluster with no masters? sounds like a cluster with no cluster.

do you have reason to believe this is reachable? (even if that's true, seems like invalid use of the cli).

Thanks for the clarification. This is an issue found by static analysis tool. I think in this case it does not harm to check the value of master_count and return NULL when it is zero.

Comment From: zuiderkwast

We can add

assert(master_count > 0);

[Edit] Also, I think the /* Can not be reached */ return NULL at the end of the function should be replaced by assert(false);. Where the function is called, we don't check for NULL, so I think it's better that it can never return NULL.