From 344e41c92228f3b8aade6220c48b7d8c6e7817a7 Mon Sep 17 00:00:00 2001
From: Binbin <binloveplay1314@qq.com>
Date: Sun, 6 Feb 2022 19:13:56 +0800
Subject: [PATCH] Fix PSYNC crash with wrong offset (#10243)

`PSYNC replicationid str_offset` will crash the server.

The reason is in `masterTryPartialResynchronization`,
we will call `getLongLongFromObjectOrReply` check the
offset. With a wrong offset, it will add a reply and
then trigger a full SYNC and the client become a replica.

So crash in `c->bufpos == 0 && listLength(c->reply) == 0`.
In this commit, we check the psync_offset before entering
function `masterTryPartialResynchronization`, and return.

Regardless of that crash, accepting the sync, but also replying
with an error would have corrupt the replication stream.
---
 src/replication.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/src/replication.c b/src/replication.c
index e387a8fd4..c389a1407 100644
--- a/src/replication.c
+++ b/src/replication.c
@@ -705,18 +705,12 @@ int replicationSetupSlaveForFullResync(client *slave, long long offset) {
  *
  * On success return C_OK, otherwise C_ERR is returned and we proceed
  * with the usual full resync. */
-int masterTryPartialResynchronization(client *c) {
-    long long psync_offset, psync_len;
+int masterTryPartialResynchronization(client *c, long long psync_offset) {
+    long long psync_len;
     char *master_replid = c->argv[1]->ptr;
     char buf[128];
     int buflen;
 
-    /* Parse the replication offset asked by the slave. Go to full sync
-     * on parse error: this should never happen but we try to handle
-     * it in a robust way compared to aborting. */
-    if (getLongLongFromObjectOrReply(c,c->argv[2],&psync_offset,NULL) !=
-       C_OK) goto need_full_resync;
-
     /* Is the replication ID of this master the same advertised by the wannabe
      * slave via PSYNC? If the replication ID changed this master has a
      * different replication history, and there is no way to continue.
@@ -977,7 +971,14 @@ void syncCommand(client *c) {
      * So the slave knows the new replid and offset to try a PSYNC later
      * if the connection with the master is lost. */
     if (!strcasecmp(c->argv[0]->ptr,"psync")) {
-        if (masterTryPartialResynchronization(c) == C_OK) {
+        long long psync_offset;
+        if (getLongLongFromObjectOrReply(c, c->argv[2], &psync_offset, NULL) != C_OK) {
+            serverLog(LL_WARNING, "Replica %s asks for synchronization but with a wrong offset",
+                      replicationGetSlaveName(c));
+            return;
+        }
+
+        if (masterTryPartialResynchronization(c, psync_offset) == C_OK) {
             server.stat_sync_partial_ok++;
             return; /* No full resync needed, return. */
         } else {