Describe the bug File: src/replication.c

In function replicationCron(), slave is freed by freeClient(slave) at line 3,404. But the freed slave is dereferenced at line 3,410 by slave->replstate.

        while((ln = listNext(&li))) {
            client *slave = ln->value;

            if (slave->replstate == SLAVE_STATE_ONLINE) {
                if (slave->flags & CLIENT_PRE_PSYNC)
                    continue;
                if ((server.unixtime - slave->repl_ack_time) > server.repl_timeout) {
                    serverLog(LL_WARNING, "Disconnecting timedout replica (streaming sync): %s",
                          replicationGetSlaveName(slave));
                    freeClient(slave);  /* ------> Freed Here */
                }
            }

            if (slave->replstate == SLAVE_STATE_WAIT_BGSAVE_END && server.rdb_child_type == RDB_CHILD_TYPE_SOCKET) {
             /* ----> Used Here!*/

It causes a use after free.

Expected behavior

After the slave is freed, the freed chunk could be allocated by other objects and rewrite the value of slave->replstate, it could cause crash or others.

I think we should set a new variable to accept the value of slave->replstate before the first if branch, and use the new variable later.

Comment From: oranagra

Thanks @Yunlongs This is new code, Wanna make a PR to solve it?

Comment From: Yunlongs

Thanks @Yunlongs This is new code, Wanna make a PR to solve it?

My network proxy is something wrong, i cannot subimt a PR now.. I need some time to fix it. Or can you just fix this bug?

Comment From: oranagra

Yeah, no worries. If you can't, we'll fix soon.