Relaxed RDB check for foreign RDB formats

src/cluster.c

@@ -166,11 +166,7 @@ int verifyDumpPayload(unsigned char *p, size_t len, uint16_t *rdbver_ptr) {
     if (rdbver_ptr) {
         *rdbver_ptr = rdbver;
     }
-    if ((rdbver >= RDB_FOREIGN_VERSION_MIN && rdbver <= RDB_FOREIGN_VERSION_MAX) ||
-        (rdbver > RDB_VERSION && server.rdb_version_check == RDB_VERSION_CHECK_STRICT)) {
-        return C_ERR;
-    }
-
+    if (!rdbIsVersionAccepted(rdbver, false, false)) return C_ERR;
     if (server.skip_checksum_validation) return C_OK;
 
     /* Verify CRC64 */
@@ -203,6 +199,7 @@ void dumpCommand(client *c) {
 /* RESTORE key ttl serialized-value [REPLACE] [ABSTTL] [IDLETIME seconds] [FREQ frequency] */
 void restoreCommand(client *c) {
     long long ttl, lfu_freq = -1, lru_idle = -1, lru_clock = -1;
+    uint16_t rdbver = 0;
     rio payload;
     int j, type, replace = 0, absttl = 0;
     robj *obj;
@@ -251,14 +248,27 @@ void restoreCommand(client *c) {
     }
 
     /* Verify RDB version and data checksum. */
-    if (verifyDumpPayload(c->argv[3]->ptr, sdslen(c->argv[3]->ptr), NULL) == C_ERR) {
+    if (verifyDumpPayload(c->argv[3]->ptr, sdslen(c->argv[3]->ptr), &rdbver) == C_ERR) {
         addReplyError(c, "DUMP payload version or checksum are wrong");
         return;
     }
 
     rioInitWithBuffer(&payload, c->argv[3]->ptr);
-    if (((type = rdbLoadObjectType(&payload)) == -1) ||
-        ((obj = rdbLoadObject(type, &payload, key->ptr, c->db->id, NULL)) == NULL)) {
+    type = rdbLoadObjectType(&payload);
+    if (type == -1) {
+        addReplyError(c, "Bad data format");
+        return;
+    }
+
+    /* If it's a foreign RDB format, only accept old data types that we know
+     * existed in the past and that don't clash with new types added later. */
+    if (rdbIsForeignVersion(rdbver) && type >= RDB_FOREIGN_TYPE_MIN) {
+        addReplyErrorFormat(c, "Unsupported foreign data type: %d", type);
+        return;
+    }
+
+    obj = rdbLoadObject(type, &payload, key->ptr, c->db->id, NULL);
+    if (obj == NULL) {
         addReplyError(c, "Bad data format");
         return;
     }

src/rdb.c

@@ -76,6 +76,22 @@ void rdbCheckError(const char *fmt, ...);
 void rdbCheckSetError(const char *fmt, ...);
 int rdbLoadRioWithLoadingCtx(rio *rdb, int rdbflags, rdbSaveInfo *rsi, rdbLoadingCtx *rdb_loading_ctx);
 
+/* Returns true if the RDB version is valid and accepted, false otherwise. This
+ * function takes configuration into account. The parameter `is_valkey_magic`
+ * indicates that an RDB file with the VALKEY magic string was parsed.
+ * `is_redis_magic` indicates a legacy RDB file with the REDIS magic string.
+ * When there is no magic string such as in DUMP/RESTORE, set both to false. */
+bool rdbIsVersionAccepted(int rdbver, bool is_valkey_magic, bool is_redis_magic) {
+    if (rdbver < 1) return false;
+    if (is_valkey_magic && rdbver <= RDB_FOREIGN_VERSION_MAX) return false;
+    if (is_redis_magic && rdbver > RDB_FOREIGN_VERSION_MAX) return false;
+    if (server.rdb_version_check == RDB_VERSION_CHECK_STRICT) {
+        if (rdbver > RDB_VERSION) return false; /* future version */
+        if (rdbIsForeignVersion(rdbver)) return false;
+    }
+    return true;
+}
+
 #ifdef __GNUC__
 void rdbReportError(int corruption_error, int linenum, char *reason, ...) __attribute__((format(printf, 3, 4)));
 #endif
@@ -2875,6 +2891,10 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) {
             return NULL;
         }
         o = createModuleObject(mt, ptr);
+    } else if (server.rdb_version_check == RDB_VERSION_CHECK_RELAXED) {
+        /* Future or foreign type. Don't report it as an internal error. */
+        if (error) *error = RDB_LOAD_ERR_UNKNOWN_TYPE;
+        return NULL;
     } else {
         rdbReportReadError("Unknown RDB encoding type %d", rdbtype);
         return NULL;
@@ -3084,10 +3104,7 @@ int rdbLoadRioWithLoadingCtx(rio *rdb, int rdbflags, rdbSaveInfo *rsi, rdbLoadin
         return C_ERR;
     }
     rdbver = atoi(buf + 6);
-    if (rdbver < 1 || (rdbver < RDB_FOREIGN_VERSION_MIN && !is_redis_magic) ||
-        (rdbver >= RDB_FOREIGN_VERSION_MIN && rdbver <= RDB_FOREIGN_VERSION_MAX) ||
-        (rdbver > RDB_FOREIGN_VERSION_MAX && !is_valkey_magic) ||
-        (rdbver > RDB_VERSION && server.rdb_version_check == RDB_VERSION_CHECK_STRICT)) {
+    if (!rdbIsVersionAccepted(rdbver, is_valkey_magic, is_redis_magic)) {
         serverLog(LL_WARNING, "Can't handle RDB format version %d", rdbver);
         return C_ERR;
     }
@@ -3103,6 +3120,13 @@ int rdbLoadRioWithLoadingCtx(rio *rdb, int rdbflags, rdbSaveInfo *rsi, rdbLoadin
         /* Read type. */
         if ((type = rdbLoadType(rdb)) == -1) goto eoferr;
 
+        /* Safeguard for unknown foreign opcode interpretations. */
+        if (is_redis_magic && type >= RDB_FOREIGN_TYPE_MIN && type <= RDB_FOREIGN_TYPE_MAX) {
+            serverLog(LL_WARNING, "Can't handle foreign type or opcode %d in RDB with version %d",
+                      type, rdbver);
+            return C_ERR;
+        }
+
         /* Handle special types. */
         if (type == RDB_OPCODE_EXPIRETIME) {
             /* EXPIRETIME: load an expire associated with the next key
@@ -3155,6 +3179,19 @@ int rdbLoadRioWithLoadingCtx(rio *rdb, int rdbflags, rdbSaveInfo *rsi, rdbLoadin
             if ((expires_size = rdbLoadLen(rdb, NULL)) == RDB_LENERR) goto eoferr;
             should_expand_db = 1;
             continue; /* Read next opcode. */
+        } else if (type == RDB_OPCODE_SLOT_INFO) {
+            /* RDB slot info size annotations used in pre-8.0 and foreign RDB.
+             * See the aux field "slot-info". */
+            uint64_t slot_id, slot_size, expires_slot_size;
+            if ((slot_id = rdbLoadLen(rdb, NULL)) == RDB_LENERR) goto eoferr;
+            if ((slot_size = rdbLoadLen(rdb, NULL)) == RDB_LENERR) goto eoferr;
+            if ((expires_slot_size = rdbLoadLen(rdb, NULL)) == RDB_LENERR) goto eoferr;
+            if (server.cluster_enabled && slot_id < CLUSTER_SLOTS) {
+                if (slot_size) kvstoreHashtableExpand(db->keys, slot_id, slot_size);
+                if (expires_slot_size) kvstoreHashtableExpand(db->expires, slot_id, expires_slot_size);
+                should_expand_db = 0;
+            }
+            continue; /* Read next opcode. */
         } else if (type == RDB_OPCODE_AUX) {
             /* AUX: generic string-string fields. Use to add state to RDB
              * which is backward compatible. Implementations of RDB loading
@@ -3215,7 +3252,7 @@ int rdbLoadRioWithLoadingCtx(rio *rdb, int rdbflags, rdbSaveInfo *rsi, rdbLoadin
                     goto eoferr;
                 }
 
-                if (server.cluster_enabled) {
+                if (server.cluster_enabled && slot_id >= 0 && slot_id < CLUSTER_SLOTS) {
                     /* In cluster mode we resize individual slot specific dictionaries based on the number of keys that
                      * slot holds. */
                     if (slot_size) kvstoreHashtableExpand(db->keys, slot_id, slot_size);
@@ -3352,6 +3389,10 @@ int rdbLoadRioWithLoadingCtx(rio *rdb, int rdbflags, rdbSaveInfo *rsi, rdbLoadin
             if (error == RDB_LOAD_ERR_EMPTY_KEY) {
                 if (empty_keys_skipped++ < 10) serverLog(LL_NOTICE, "rdbLoadObject skipping empty key: %s", key);
                 sdsfree(key);
+            } else if (error == RDB_LOAD_ERR_UNKNOWN_TYPE) {
+                sdsfree(key);
+                serverLog(LL_WARNING, "Unknown type or opcode when loading DB. Unrecoverable error, aborting now.");
+                return C_ERR;
             } else {
                 sdsfree(key);
                 goto eoferr;

src/rdb.h

@@ -57,6 +57,10 @@
 static_assert(RDB_VERSION < RDB_FOREIGN_VERSION_MIN || RDB_VERSION > RDB_FOREIGN_VERSION_MAX,
               "RDB version in foreign version range");
 
+static inline bool rdbIsForeignVersion(int rdbver) {
+    return rdbver >= RDB_FOREIGN_VERSION_MIN && rdbver <= RDB_FOREIGN_VERSION_MAX;
+}
+
 /* Defines related to the dump file format. To store 32 bits lengths for short
  * keys requires a lot of space, so we check the most significant 2 bits of
  * the first byte to interpreter the length:
@@ -112,11 +116,16 @@ enum RdbType {
     RDB_TYPE_STREAM_LISTPACKS_2 = 19,
     RDB_TYPE_SET_LISTPACK = 20,
     RDB_TYPE_STREAM_LISTPACKS_3 = 21,
-    RDB_TYPE_HASH_2 = 22,
+    RDB_TYPE_HASH_2 = 22, /* Hash with field-level expiration (Valkey 9.0) */
     RDB_TYPE_LAST
 };
 /* NOTE: WHEN ADDING NEW RDB TYPE, UPDATE rdb_type_string[] */
 
+/* When our RDB format diverges, we need to reject types/opcodes for which we
+ * may have assigned a different meaning compared to other implementations. */
+#define RDB_FOREIGN_TYPE_MIN 22
+#define RDB_FOREIGN_TYPE_MAX 243
+
 typedef int (*ChildSnapshotFunc)(int req, rio *rdb, void *privdata);
 typedef struct rdbSnapshotOptions {
     int connsnum;                    /* Number of connections. */
@@ -131,7 +140,9 @@ typedef struct rdbSnapshotOptions {
 /* Test if a type is an object type. */
 #define rdbIsObjectType(t) (((t) >= 0 && (t) <= 7) || ((t) >= 9 && (t) < RDB_TYPE_LAST))
 
-/* Special RDB opcodes (saved/loaded with rdbSaveType/rdbLoadType). */
+/* Special RDB opcodes (saved/loaded with rdbSaveType/rdbLoadType).
+ * These are special RDB types, but they start from 255 and grow down. */
+#define RDB_OPCODE_SLOT_INFO 244       /* Foreign slot info, safe to ignore. */
 #define RDB_OPCODE_FUNCTION2 245       /* function library data */
 #define RDB_OPCODE_FUNCTION_PRE_GA 246 /* old function library data for 7.0 rc1 and rc2 */
 #define RDB_OPCODE_MODULE_AUX 247      /* Module auxiliary data. */
@@ -168,9 +179,11 @@ typedef struct rdbSnapshotOptions {
 
 /* When rdbLoadObject() returns NULL, the err flag is
  * set to hold the type of error that occurred */
-#define RDB_LOAD_ERR_EMPTY_KEY 1 /* Error of empty key */
-#define RDB_LOAD_ERR_OTHER 2     /* Any other errors */
+#define RDB_LOAD_ERR_EMPTY_KEY 1    /* Error of empty key */
+#define RDB_LOAD_ERR_UNKNOWN_TYPE 2 /* Unknown type in file */
+#define RDB_LOAD_ERR_OTHER 3        /* Any other errors */
 
+bool rdbIsVersionAccepted(int rdbver, bool is_valkey_magic, bool is_redis_magic);
 ssize_t rdbWriteRaw(rio *rdb, void *p, size_t len);
 int rdbSaveType(rio *rdb, unsigned char type);
 int rdbLoadType(rio *rdb);

src/valkey-check-rdb.c

@@ -76,6 +76,7 @@ struct {
     rio *rio;
     robj *key;                     /* Current key we are reading. */
     int key_type;                  /* Current key type if != -1. */
+    int rdbver;                    /* RDB version. */
     unsigned long keys;            /* Number of keys processed. */
     unsigned long expires;         /* Number of keys with an expire. */
     unsigned long already_expired; /* Number of keys already expired. */
@@ -625,13 +626,17 @@ int redis_check_rdb(char *rdbfilename, FILE *fp) {
         goto err;
     }
     rdbver = atoi(buf + 6);
-    if (rdbver < 1 || rdbver > RDB_VERSION ||
+    if (rdbver < 1 ||
         (rdbver < RDB_FOREIGN_VERSION_MIN && !is_redis_magic) ||
-        (rdbver >= RDB_FOREIGN_VERSION_MIN && rdbver <= RDB_FOREIGN_VERSION_MAX) ||
         (rdbver > RDB_FOREIGN_VERSION_MAX && !is_valkey_magic)) {
         rdbCheckError("Can't handle RDB format version %d", rdbver);
         goto err;
+    } else if (rdbver > RDB_VERSION) {
+        rdbCheckInfo("Future RDB version %d detected", rdbver);
+    } else if (rdbIsForeignVersion(rdbver)) {
+        rdbCheckInfo("Foreign RDB version %d detected", rdbver);
     }
+    rdbstate.rdbver = rdbver;
 
     expiretime = -1;
     while (1) {
@@ -689,6 +694,12 @@ int redis_check_rdb(char *rdbfilename, FILE *fp) {
             if ((db_size = rdbLoadLen(&rdb, NULL)) == RDB_LENERR) goto eoferr;
             if ((expires_size = rdbLoadLen(&rdb, NULL)) == RDB_LENERR) goto eoferr;
             continue; /* Read type again. */
+        } else if (type == RDB_OPCODE_SLOT_INFO) {
+            /* Hint used in foreign RDB versions. */
+            if (rdbLoadLen(&rdb, NULL) == RDB_LENERR) goto eoferr;
+            if (rdbLoadLen(&rdb, NULL) == RDB_LENERR) goto eoferr;
+            if (rdbLoadLen(&rdb, NULL) == RDB_LENERR) goto eoferr;
+            continue; /* Read type again. */
         } else if (type == RDB_OPCODE_AUX) {
             /* AUX: generic string-string fields. Use to add state to RDB
              * which is backward compatible. Implementations of RDB loading
@@ -742,11 +753,19 @@ int redis_check_rdb(char *rdbfilename, FILE *fp) {
             }
             rdbstate.functions_num++;
             continue;
-        } else {
-            if (!rdbIsObjectType(type)) {
+        } else if (rdbIsForeignVersion(rdbver) &&
+                   type >= RDB_FOREIGN_TYPE_MIN &&
+                   type <= RDB_FOREIGN_TYPE_MAX) {
+            rdbCheckError("Unknown object type %d in RDB file with foreign version %d", type, rdbver);
+            goto err;
+        } else if (!rdbIsObjectType(type)) {
+            if (rdbver > RDB_VERSION) {
+                rdbCheckError("Unknown object type %d in RDB file with future version %d", type, rdbver);
+            } else {
                 rdbCheckError("Invalid object type: %d", type);
-                goto err;
             }
+            goto err;
+        } else {
             rdbstate.key_type = type;
         }
 
@@ -894,8 +913,13 @@ int redis_check_rdb_main(int argc, char **argv, FILE *fp) {
     rdbCheckInfo("Checking RDB file %s", argv[1]);
     rdbCheckSetupSignals();
     int retval = redis_check_rdb(argv[1], fp);
+    rdbCheckInfo("Check RDB returned error code %d (0 means success)", retval);
     if (retval == 0) {
-        rdbCheckInfo("\\o/ RDB looks OK! \\o/");
+        if (rdbIsForeignVersion(rdbstate.rdbver) || rdbstate.rdbver > RDB_VERSION) {
+            rdbCheckInfo("\\o/ RDB looks OK, but loading requires config 'rdb-version-check relaxed'");
+        } else {
+            rdbCheckInfo("\\o/ RDB looks OK! \\o/");
+        }
         rdbShowGenericInfo();
     }
     if (fp) return (retval == 0) ? C_OK : C_ERR;

tests/integration/rdb.tcl

@@ -15,7 +15,10 @@ set server_path [tmpdir "server.rdb-encoding-test"]
 
 # Copy RDB with different encodings in server path
 exec cp tests/assets/encodings.rdb $server_path
+exec cp tests/assets/encodings-rdb12.rdb $server_path
+exec cp tests/assets/encodings-rdb75-unknown-types.rdb $server_path
 exec cp tests/assets/encodings-rdb987.rdb $server_path
+exec cp tests/assets/encodings-rdb987-unknown-types.rdb $server_path
 exec cp tests/assets/list-quicklist.rdb $server_path
 
 start_server [list overrides [list "dir" $server_path "dbfilename" "list-quicklist.rdb" save ""]] {
@@ -55,7 +58,7 @@ start_server_and_kill_it [list "dir" $server_path "dbfilename" "encodings-rdb987
             [string match {*Fatal error loading*} \
                  [exec tail -1 < [dict get $srv stdout]]]
         } else {
-            fail "Server started even if RDB version check failed"
+            fail "Server started even though RDB version is unsupported"
         }
     }
 }
@@ -69,6 +72,42 @@ start_server [list overrides [list "dir" $server_path \
     } $csv_dump
 }
 
+start_server_and_kill_it [list dir $server_path \
+                              dbfilename "encodings-rdb987-unknown-types.rdb" \
+                              rdb-version-check relaxed] {
+    test "RDB future version loading with unknown types, relaxed version check" {
+        wait_for_condition 50 100 {
+            [string match {*Unknown type or opcode when loading DB. Unrecoverable error, aborting now.*} \
+                 [exec tail -2 < [dict get $srv stdout]]]
+        } else {
+            fail "Server started even though RDB contains unknown types"
+        }
+    }
+}
+
+start_server [list overrides [list dir $server_path \
+                                  dbfilename "encodings-rdb12.rdb" \
+                                  rdb-version-check relaxed]] {
+    test "RDB foreign version loading, relaxed version check" {
+        r select 0
+        assert_equal foo [r keys *]
+        assert_equal bar [r get foo]
+    }
+}
+
+start_server_and_kill_it [list dir $server_path \
+                              dbfilename "encodings-rdb75-unknown-types.rdb" \
+                              rdb-version-check relaxed] {
+    test "RDB foreign version loading with unknown types, relaxed version check" {
+        wait_for_condition 50 100 {
+            [string match {*Can't handle foreign type or opcode 150 in RDB with version 75*} \
+                 [exec tail -2 < [dict get $srv stdout]]]
+        } else {
+            fail "Server started even though RDB contains unknown types"
+        }
+    }
+}
+
 set server_path [tmpdir "server.rdb-startup-test"]
 
 start_server [list overrides [list "dir" $server_path] keep_persistence true] {