diff --git a/redis.conf b/redis.conf index b11459b26..13766171e 100644 --- a/redis.conf +++ b/redis.conf @@ -366,6 +366,21 @@ rdbcompression yes # tell the loading code to skip the check. rdbchecksum yes +# Enables or disables full sanitation checks for ziplist and listpack etc when +# loading an RDB or RESTORE payload. This reduces the chances of a assertion or +# crash later on while processing commands. +# Options: +# no - Never perform full sanitation +# yes - Always perform full sanitation +# clients - Perform full sanitation only for user connections. +# Excludes: RDB files, RESTORE commands received from the master +# connection, and client connections which have the +# skip-sanitize-payload ACL flag. +# The default should be 'clients' but since it currently affects cluster +# resharding via MIGRATE, it is temporarily set to 'no' by default. +# +# sanitize-dump-payload no + # The filename where to dump the DB dbfilename dump.rdb @@ -725,6 +740,8 @@ replica-priority 100 # off Disable the user: it's no longer possible to authenticate # with this user, however the already authenticated connections # will still work. +# skip-sanitize-payload RESTORE dump-payload sanitation is skipped. +# sanitize-payload RESTORE dump-payload is sanitized (default). # + Allow the execution of that command # - Disallow the execution of that command # +@ Allow the execution of all the commands in such category diff --git a/src/acl.c b/src/acl.c index e9acc11a9..a1a7c4237 100644 --- a/src/acl.c +++ b/src/acl.c @@ -89,12 +89,15 @@ struct ACLUserFlag { const char *name; uint64_t flag; } ACLUserFlags[] = { + /* Note: the order here dictates the emitted order at ACLDescribeUser */ {"on", USER_FLAG_ENABLED}, {"off", USER_FLAG_DISABLED}, {"allkeys", USER_FLAG_ALLKEYS}, {"allchannels", USER_FLAG_ALLCHANNELS}, {"allcommands", USER_FLAG_ALLCOMMANDS}, {"nopass", USER_FLAG_NOPASS}, + {"skip-sanitize-payload", USER_FLAG_SANITIZE_PAYLOAD_SKIP}, + {"sanitize-payload", USER_FLAG_SANITIZE_PAYLOAD}, {NULL,0} /* Terminator. */ }; @@ -829,6 +832,12 @@ int ACLSetUser(user *u, const char *op, ssize_t oplen) { } else if (!strcasecmp(op,"off")) { u->flags |= USER_FLAG_DISABLED; u->flags &= ~USER_FLAG_ENABLED; + } else if (!strcasecmp(op,"skip-sanitize-payload")) { + u->flags |= USER_FLAG_SANITIZE_PAYLOAD_SKIP; + u->flags &= ~USER_FLAG_SANITIZE_PAYLOAD; + } else if (!strcasecmp(op,"sanitize-payload")) { + u->flags &= ~USER_FLAG_SANITIZE_PAYLOAD_SKIP; + u->flags |= USER_FLAG_SANITIZE_PAYLOAD; } else if (!strcasecmp(op,"allkeys") || !strcasecmp(op,"~*")) { @@ -1004,6 +1013,7 @@ int ACLSetUser(user *u, const char *op, ssize_t oplen) { serverAssert(ACLSetUser(u,"resetkeys",-1) == C_OK); serverAssert(ACLSetUser(u,"resetchannels",-1) == C_OK); serverAssert(ACLSetUser(u,"off",-1) == C_OK); + serverAssert(ACLSetUser(u,"sanitize-payload",-1) == C_OK); serverAssert(ACLSetUser(u,"-@all",-1) == C_OK); } else { errno = EINVAL; diff --git a/src/config.c b/src/config.c index 84b218cd7..5fdde44cd 100644 --- a/src/config.c +++ b/src/config.c @@ -119,6 +119,13 @@ configEnum acl_pubsub_default_enum[] = { {NULL, 0} }; +configEnum sanitize_dump_payload_enum[] = { + {"no", SANITIZE_DUMP_NO}, + {"yes", SANITIZE_DUMP_YES}, + {"clients", SANITIZE_DUMP_CLIENTS}, + {NULL, 0} +}; + /* Output buffer limits presets. */ clientBufferLimitsConfig clientBufferLimitsDefaults[CLIENT_TYPE_OBUF_COUNT] = { {0, 0, 0}, /* normal */ @@ -2361,6 +2368,7 @@ standardConfig configs[] = { createEnumConfig("appendfsync", NULL, MODIFIABLE_CONFIG, aof_fsync_enum, server.aof_fsync, AOF_FSYNC_EVERYSEC, NULL, NULL), createEnumConfig("oom-score-adj", NULL, MODIFIABLE_CONFIG, oom_score_adj_enum, server.oom_score_adj, OOM_SCORE_ADJ_NO, NULL, updateOOMScoreAdj), createEnumConfig("acl-pubsub-default", NULL, MODIFIABLE_CONFIG, acl_pubsub_default_enum, server.acl_pubusub_default, USER_FLAG_ALLCHANNELS, NULL, NULL), + createEnumConfig("sanitize-dump-payload", NULL, MODIFIABLE_CONFIG, sanitize_dump_payload_enum, server.sanitize_dump_payload, SANITIZE_DUMP_NO, NULL, NULL), /* Integer configs */ createIntConfig("databases", NULL, IMMUTABLE_CONFIG, 1, INT_MAX, server.dbnum, 16, INTEGER_CONFIG, NULL, NULL), diff --git a/src/defrag.c b/src/defrag.c index 04ade30ea..165e55f65 100644 --- a/src/defrag.c +++ b/src/defrag.c @@ -5,7 +5,6 @@ * We do that by scanning the keyspace and for each pointer we have, we can try to * ask the allocator if moving it to a new address will help reduce fragmentation. * - * Copyright (c) 2020, Oran Agra * Copyright (c) 2020, Redis Labs, Inc * All rights reserved. * diff --git a/src/intset.c b/src/intset.c index 4445a5ca6..0079199cf 100644 --- a/src/intset.c +++ b/src/intset.c @@ -281,6 +281,33 @@ size_t intsetBlobLen(intset *is) { return sizeof(intset)+intrev32ifbe(is->length)*intrev32ifbe(is->encoding); } +int intsetValidateIntegrity(const unsigned char *p, size_t size) { + const intset *is = (const intset *)p; + /* check that we can actually read the header. */ + if (size < sizeof(*is)) + return 0; + + uint32_t encoding = intrev32ifbe(is->encoding); + + size_t record_size; + if (encoding == INTSET_ENC_INT64) { + record_size = INTSET_ENC_INT64; + } else if (encoding == INTSET_ENC_INT32) { + record_size = INTSET_ENC_INT32; + } else if (encoding == INTSET_ENC_INT16){ + record_size = INTSET_ENC_INT16; + } else { + return 0; + } + + /* check that the size matchies (all records are inside the buffer). */ + uint32_t count = intrev32ifbe(is->length); + if (sizeof(*is) + count*record_size != size) + return 0; + + return 1; +} + #ifdef REDIS_TEST #include #include diff --git a/src/intset.h b/src/intset.h index 8119e6636..4fba339d5 100644 --- a/src/intset.h +++ b/src/intset.h @@ -46,6 +46,7 @@ int64_t intsetRandom(intset *is); uint8_t intsetGet(intset *is, uint32_t pos, int64_t *value); uint32_t intsetLen(const intset *is); size_t intsetBlobLen(intset *is); +int intsetValidateIntegrity(const unsigned char *is, size_t size); #ifdef REDIS_TEST int intsetTest(int argc, char *argv[]); diff --git a/src/listpack.c b/src/listpack.c index f8c34429e..580954075 100644 --- a/src/listpack.c +++ b/src/listpack.c @@ -5,6 +5,7 @@ * https://github.com/antirez/listpack * * Copyright (c) 2017, Salvatore Sanfilippo + * Copyright (c) 2020, Redis Labs, Inc * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -41,6 +42,7 @@ #include "listpack.h" #include "listpack_malloc.h" +#include "redisassert.h" #define LP_HDR_SIZE 6 /* 32 bit total len + 16 bit number of elements. */ #define LP_HDR_NUMELE_UNKNOWN UINT16_MAX @@ -114,6 +116,21 @@ (p)[5] = ((v)>>8)&0xff; \ } while(0) +/* Validates that 'p' is not ouside the listpack. + * All function that return a pointer to an element in the listpack will assert + * that this element is valid, so it can be freely used. + * Generally functions such lpNext and lpDelete assume the input pointer is + * already validated (since it's the return value of another function). */ +#define ASSERT_INTEGRITY(lp, p) do { \ + assert((p) >= (lp)+LP_HDR_SIZE && (p) < (lp)+lpGetTotalBytes((lp))); \ +} while (0) + +/* Similar to the above, but validates the entire element lenth rather than just + * it's pointer. */ +#define ASSERT_INTEGRITY_LEN(lp, p, len) do { \ + assert((p) >= (lp)+LP_HDR_SIZE && (p)+(len) < (lp)+lpGetTotalBytes((lp))); \ +} while (0) + /* Convert a string into a signed 64 bit integer. * The function returns 1 if the string could be parsed into a (non-overflowing) * signed 64 bit int, 0 otherwise. The 'value' will be set to the parsed value @@ -367,9 +384,14 @@ void lpEncodeString(unsigned char *buf, unsigned char *s, uint32_t len) { } } -/* Return the encoded length of the listpack element pointed by 'p'. If the - * element encoding is wrong then 0 is returned. */ -uint32_t lpCurrentEncodedSize(unsigned char *p) { +/* Return the encoded length of the listpack element pointed by 'p'. + * This includes the encoding byte, length bytes, and the element data itself. + * If the element encoding is wrong then 0 is returned. + * Note that this method may access additional bytes (in case of 12 and 32 bit + * str), so should only be called when we know 'p' was already validated by + * lpCurrentEncodedSizeBytes or ASSERT_INTEGRITY_LEN (possibly since 'p' is + * a return value of another function that validated its return. */ +uint32_t lpCurrentEncodedSizeUnsafe(unsigned char *p) { if (LP_ENCODING_IS_7BIT_UINT(p[0])) return 1; if (LP_ENCODING_IS_6BIT_STR(p[0])) return 1+LP_ENCODING_6BIT_STR_LEN(p); if (LP_ENCODING_IS_13BIT_INT(p[0])) return 2; @@ -383,12 +405,30 @@ uint32_t lpCurrentEncodedSize(unsigned char *p) { return 0; } +/* Return bytes needed to encode the length of the listpack element pointed by 'p'. + * This includes just the encodign byte, and the bytes needed to encode the length + * of the element (excluding the element data itself) + * If the element encoding is wrong then 0 is returned. */ +uint32_t lpCurrentEncodedSizeBytes(unsigned char *p) { + if (LP_ENCODING_IS_7BIT_UINT(p[0])) return 1; + if (LP_ENCODING_IS_6BIT_STR(p[0])) return 1; + if (LP_ENCODING_IS_13BIT_INT(p[0])) return 1; + if (LP_ENCODING_IS_16BIT_INT(p[0])) return 1; + if (LP_ENCODING_IS_24BIT_INT(p[0])) return 1; + if (LP_ENCODING_IS_32BIT_INT(p[0])) return 1; + if (LP_ENCODING_IS_64BIT_INT(p[0])) return 1; + if (LP_ENCODING_IS_12BIT_STR(p[0])) return 2; + if (LP_ENCODING_IS_32BIT_STR(p[0])) return 5; + if (p[0] == LP_EOF) return 1; + return 0; +} + /* Skip the current entry returning the next. It is invalid to call this * function if the current element is the EOF element at the end of the * listpack, however, while this function is used to implement lpNext(), * it does not return NULL when the EOF element is encountered. */ unsigned char *lpSkip(unsigned char *p) { - unsigned long entrylen = lpCurrentEncodedSize(p); + unsigned long entrylen = lpCurrentEncodedSizeUnsafe(p); entrylen += lpEncodeBacklen(NULL,entrylen); p += entrylen; return p; @@ -398,8 +438,9 @@ unsigned char *lpSkip(unsigned char *p) { * the pointer to the next element (the one on the right), or NULL if 'p' * already pointed to the last element of the listpack. */ unsigned char *lpNext(unsigned char *lp, unsigned char *p) { - ((void) lp); /* lp is not used for now. However lpPrev() uses it. */ + assert(p); p = lpSkip(p); + ASSERT_INTEGRITY(lp, p); if (p[0] == LP_EOF) return NULL; return p; } @@ -408,11 +449,14 @@ unsigned char *lpNext(unsigned char *lp, unsigned char *p) { * the pointer to the previous element (the one on the left), or NULL if 'p' * already pointed to the first element of the listpack. */ unsigned char *lpPrev(unsigned char *lp, unsigned char *p) { + assert(p); if (p-lp == LP_HDR_SIZE) return NULL; p--; /* Seek the first backlen byte of the last element. */ uint64_t prevlen = lpDecodeBacklen(p); prevlen += lpEncodeBacklen(NULL,prevlen); - return p-prevlen+1; /* Seek the first byte of the previous entry. */ + p -= prevlen-1; /* Seek the first byte of the previous entry. */ + ASSERT_INTEGRITY(lp, p); + return p; } /* Return a pointer to the first element of the listpack, or NULL if the @@ -570,8 +614,7 @@ unsigned char *lpGet(unsigned char *p, int64_t *count, unsigned char *intbuf) { /* Insert, delete or replace the specified element 'ele' of length 'len' at * the specified position 'p', with 'p' being a listpack element pointer - * obtained with lpFirst(), lpLast(), lpIndex(), lpNext(), lpPrev() or - * lpSeek(). + * obtained with lpFirst(), lpLast(), lpNext(), lpPrev() or lpSeek(). * * The element is inserted before, after, or replaces the element pointed * by 'p' depending on the 'where' argument, that can be LP_BEFORE, LP_AFTER @@ -610,6 +653,7 @@ unsigned char *lpInsert(unsigned char *lp, unsigned char *ele, uint32_t size, un if (where == LP_AFTER) { p = lpSkip(p); where = LP_BEFORE; + ASSERT_INTEGRITY(lp, p); } /* Store the offset of the element 'p', so that we can obtain its @@ -639,8 +683,9 @@ unsigned char *lpInsert(unsigned char *lp, unsigned char *ele, uint32_t size, un uint64_t old_listpack_bytes = lpGetTotalBytes(lp); uint32_t replaced_len = 0; if (where == LP_REPLACE) { - replaced_len = lpCurrentEncodedSize(p); + replaced_len = lpCurrentEncodedSizeUnsafe(p); replaced_len += lpEncodeBacklen(NULL,replaced_len); + ASSERT_INTEGRITY_LEN(lp, p, replaced_len); } uint64_t new_listpack_bytes = old_listpack_bytes + enclen + backlen_size @@ -801,3 +846,86 @@ unsigned char *lpSeek(unsigned char *lp, long index) { } } +/* Validate the integrity of a single listpack entry and move to the next one. + * The input argument 'pp' is a reference to the current record and is advanced on exit. + * Returns 1 if valid, 0 if invalid. */ +int lpValidateNext(unsigned char *lp, unsigned char **pp, size_t lpbytes) { +#define OUT_OF_RANGE(p) ( \ + (p) < lp + LP_HDR_SIZE || \ + (p) > lp + lpbytes - 1) + unsigned char *p = *pp; + if (!p) + return 0; + + if (*p == LP_EOF) { + *pp = NULL; + return 1; + } + + /* check that we can read the encoded size */ + uint32_t lenbytes = lpCurrentEncodedSizeBytes(p); + if (!lenbytes) + return 0; + + /* make sure the encoded entry length doesn't rech outside the edge of the listpack */ + if (OUT_OF_RANGE(p + lenbytes)) + return 0; + + /* get the entry length and encoded backlen. */ + unsigned long entrylen = lpCurrentEncodedSizeUnsafe(p); + unsigned long encodedBacklen = lpEncodeBacklen(NULL,entrylen); + entrylen += encodedBacklen; + + /* make sure the entry doesn't rech outside the edge of the listpack */ + if (OUT_OF_RANGE(p + entrylen)) + return 0; + + /* move to the next entry */ + p += entrylen; + + /* make sure the encoded length at the end patches the one at the beginning. */ + uint64_t prevlen = lpDecodeBacklen(p-1); + if (prevlen + encodedBacklen != entrylen) + return 0; + + *pp = p; + return 1; +#undef OUT_OF_RANGE +} + +/* Validate the integrity of the data stracture. + * when `deep` is 0, only the integrity of the header is validated. + * when `deep` is 1, we scan all the entries one by one. */ +int lpValidateIntegrity(unsigned char *lp, size_t size, int deep){ + /* Check that we can actually read the header. (and EOF) */ + if (size < LP_HDR_SIZE + 1) + return 0; + + /* Check that the encoded size in the header must match the allocated size. */ + size_t bytes = lpGetTotalBytes(lp); + if (bytes != size) + return 0; + + /* The last byte must be the terminator. */ + if (lp[size-1] != LP_EOF) + return 0; + + if (!deep) + return 1; + + /* Validate the invividual entries. */ + uint32_t count = 0; + unsigned char *p = lpFirst(lp); + while(p) { + if (!lpValidateNext(lp, &p, bytes)) + return 0; + count++; + } + + /* Check that the count in the header is correct */ + uint32_t numele = lpGetNumElements(lp); + if (numele != LP_HDR_NUMELE_UNKNOWN && numele != count) + return 0; + + return 1; +} diff --git a/src/listpack.h b/src/listpack.h index af67b4b41..e8375628b 100644 --- a/src/listpack.h +++ b/src/listpack.h @@ -57,5 +57,7 @@ unsigned char *lpNext(unsigned char *lp, unsigned char *p); unsigned char *lpPrev(unsigned char *lp, unsigned char *p); uint32_t lpBytes(unsigned char *lp); unsigned char *lpSeek(unsigned char *lp, long index); +int lpValidateIntegrity(unsigned char *lp, size_t size, int deep); +int lpValidateNext(unsigned char *lp, unsigned char **pp, size_t lpbytes); #endif diff --git a/src/rdb.c b/src/rdb.c index e3f34b3f0..af527f94c 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -68,17 +68,27 @@ void rdbReportError(int corruption_error, int linenum, char *reason, ...) { vsnprintf(msg+len,sizeof(msg)-len,reason,ap); va_end(ap); - if (!rdbCheckMode) { - if (rdbFileBeingLoaded || corruption_error) { - serverLog(LL_WARNING, "%s", msg); - char *argv[2] = {"",rdbFileBeingLoaded}; - redis_check_rdb_main(2,argv,NULL); - } else { - serverLog(LL_WARNING, "%s. Failure loading rdb format from socket, assuming connection error, resuming operation.", msg); - return; - } - } else { + if (!server.loading) { + /* If we're in the context of a RESTORE command, just propagate the error. */ + /* log in VERBOSE, and return (don't exit). */ + serverLog(LL_VERBOSE, "%s", msg); + return; + } else if (rdbCheckMode) { + /* If we're inside the rdb checker, let it handle the error. */ rdbCheckError("%s",msg); + } else if (rdbFileBeingLoaded) { + /* If we're loading an rdb file form disk, run rdb check (and exit) */ + serverLog(LL_WARNING, "%s", msg); + char *argv[2] = {"",rdbFileBeingLoaded}; + redis_check_rdb_main(2,argv,NULL); + } else if (corruption_error) { + /* In diskless loading, in case of corrupt file, log and exit. */ + serverLog(LL_WARNING, "%s. Failure loading rdb format", msg); + } else { + /* In diskless loading, in case of a short read (not a corrupt + * file), log and proceed (don't exit). */ + serverLog(LL_WARNING, "%s. Failure loading rdb format from socket, assuming connection error, resuming operation.", msg); + return; } serverLog(LL_WARNING, "Terminating server after rdb file reading failure."); exit(1); @@ -1489,6 +1499,17 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key) { uint64_t len; unsigned int i; + int deep_integrity_validation = server.sanitize_dump_payload == SANITIZE_DUMP_YES; + if (server.sanitize_dump_payload == SANITIZE_DUMP_CLIENTS) { + /* Skip sanitization when loading (an RDB), or getting a RESTORE command + * from either the master or a client using an ACL user with the skip-sanitize-payload flag. */ + int skip = server.loading || + (server.current_client && (server.current_client->flags & CLIENT_MASTER)); + if (!skip && server.current_client && server.current_client->user) + skip = !!(server.current_client->user->flags & USER_FLAG_SANITIZE_PAYLOAD_SKIP); + deep_integrity_validation = !skip; + } + if (rdbtype == RDB_TYPE_STRING) { /* Read string value */ if ((o = rdbLoadEncodedStringObject(rdb)) == NULL) return NULL; @@ -1685,12 +1706,20 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key) { server.list_compress_depth); while (len--) { + size_t encoded_len; unsigned char *zl = - rdbGenericLoadStringObject(rdb,RDB_LOAD_PLAIN,NULL); + rdbGenericLoadStringObject(rdb,RDB_LOAD_PLAIN,&encoded_len); if (zl == NULL) { decrRefCount(o); return NULL; } + if (deep_integrity_validation) server.stat_dump_payload_sanitizations++; + if (!ziplistValidateIntegrity(zl, encoded_len, deep_integrity_validation)) { + rdbExitReportCorruptRDB("Ziplist integrity check failed."); + decrRefCount(o); + zfree(zl); + return NULL; + } quicklistAppendZiplist(o->ptr, zl); } } else if (rdbtype == RDB_TYPE_HASH_ZIPMAP || @@ -1699,9 +1728,32 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key) { rdbtype == RDB_TYPE_ZSET_ZIPLIST || rdbtype == RDB_TYPE_HASH_ZIPLIST) { + size_t encoded_len; unsigned char *encoded = - rdbGenericLoadStringObject(rdb,RDB_LOAD_PLAIN,NULL); + rdbGenericLoadStringObject(rdb,RDB_LOAD_PLAIN,&encoded_len); if (encoded == NULL) return NULL; + if (rdbtype == RDB_TYPE_HASH_ZIPMAP) { + /* Since we don't keep zipmaps anymore, the rdb loading for these + * is O(n) anyway, use `deep` validation. */ + if (!zipmapValidateIntegrity(encoded, encoded_len, 1)) { + rdbExitReportCorruptRDB("Zipmap integrity check failed."); + zfree(encoded); + return NULL; + } + } else if (rdbtype == RDB_TYPE_SET_INTSET) { + if (!intsetValidateIntegrity(encoded, encoded_len)) { + rdbExitReportCorruptRDB("Intset integrity check failed."); + zfree(encoded); + return NULL; + } + } else { /* ziplist */ + if (deep_integrity_validation) server.stat_dump_payload_sanitizations++; + if (!ziplistValidateIntegrity(encoded, encoded_len, deep_integrity_validation)) { + rdbExitReportCorruptRDB("Ziplist integrity check failed."); + zfree(encoded); + return NULL; + } + } o = createObject(OBJ_STRING,encoded); /* Obj type fixed below. */ /* Fix the object encoding, and make sure to convert the encoded @@ -1794,14 +1846,24 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key) { } /* Load the listpack. */ + size_t lp_size; unsigned char *lp = - rdbGenericLoadStringObject(rdb,RDB_LOAD_PLAIN,NULL); + rdbGenericLoadStringObject(rdb,RDB_LOAD_PLAIN,&lp_size); if (lp == NULL) { rdbReportReadError("Stream listpacks loading failed."); sdsfree(nodekey); decrRefCount(o); return NULL; } + if (deep_integrity_validation) server.stat_dump_payload_sanitizations++; + if (!streamValidateListpackIntegrity(lp, lp_size, deep_integrity_validation)) { + rdbExitReportCorruptRDB("Stream listpack integrity check failed."); + sdsfree(nodekey); + decrRefCount(o); + zfree(lp); + return NULL; + } + unsigned char *first = lpFirst(lp); if (first == NULL) { /* Serialized listpacks should never be empty, since on @@ -2389,6 +2451,7 @@ int rdbLoadRio(rio *rdb, int rdbflags, rdbSaveInfo *rsi) { (unsigned long long)expected, (unsigned long long)cksum); rdbExitReportCorruptRDB("RDB CRC error"); + return C_ERR; } } } diff --git a/src/redis-check-rdb.c b/src/redis-check-rdb.c index f399864dd..79dbf3229 100644 --- a/src/redis-check-rdb.c +++ b/src/redis-check-rdb.c @@ -372,6 +372,7 @@ int redis_check_rdb_main(int argc, char **argv, FILE *fp) { if (shared.integers[0] == NULL) createSharedObjects(); server.loading_process_events_interval_bytes = 0; + server.sanitize_dump_payload = SANITIZE_DUMP_YES; rdbCheckMode = 1; rdbCheckInfo("Checking RDB file %s", argv[1]); rdbCheckSetupSignals(); diff --git a/src/server.c b/src/server.c index c30577da4..1b6f71aef 100644 --- a/src/server.c +++ b/src/server.c @@ -2934,6 +2934,7 @@ void resetServerStats(void) { atomicSet(server.stat_net_input_bytes, 0); atomicSet(server.stat_net_output_bytes, 0); server.stat_unexpected_error_replies = 0; + server.stat_dump_payload_sanitizations = 0; server.aof_delayed_fsync = 0; server.blocked_last_cron = 0; } @@ -4654,6 +4655,7 @@ sds genRedisInfoString(const char *section) { "tracking_total_items:%lld\r\n" "tracking_total_prefixes:%lld\r\n" "unexpected_error_replies:%lld\r\n" + "dump_payload_sanitizations:%lld\r\n" "total_reads_processed:%lld\r\n" "total_writes_processed:%lld\r\n" "io_threaded_reads_processed:%lld\r\n" @@ -4689,6 +4691,7 @@ sds genRedisInfoString(const char *section) { (unsigned long long) trackingGetTotalItems(), (unsigned long long) trackingGetTotalPrefixes(), server.stat_unexpected_error_replies, + server.stat_dump_payload_sanitizations, stat_total_reads_processed, stat_total_writes_processed, server.stat_io_reads_processed, diff --git a/src/server.h b/src/server.h index 5bbe7876c..8a35d48df 100644 --- a/src/server.h +++ b/src/server.h @@ -378,6 +378,11 @@ extern int configOOMScoreAdjValuesDefaults[CONFIG_OOM_COUNT]; #define TLS_CLIENT_AUTH_YES 1 #define TLS_CLIENT_AUTH_OPTIONAL 2 +/* Sanitize dump payload */ +#define SANITIZE_DUMP_NO 0 +#define SANITIZE_DUMP_YES 1 +#define SANITIZE_DUMP_CLIENTS 2 + /* Sets operations codes */ #define SET_OP_UNION 0 #define SET_OP_DIFF 1 @@ -781,6 +786,12 @@ typedef struct readyList { authenticated. */ #define USER_FLAG_ALLCHANNELS (1<<5) /* The user can mention any Pub/Sub channel. */ +#define USER_FLAG_SANITIZE_PAYLOAD (1<<6) /* The user require a deep RESTORE + * payload sanitization. */ +#define USER_FLAG_SANITIZE_PAYLOAD_SKIP (1<<7) /* The user should skip the + * deep sanitization of RESTORE + * payload. */ + typedef struct { sds name; /* The username as an SDS string. */ uint64_t flags; /* See USER_FLAG_* */ @@ -1213,6 +1224,7 @@ struct redisServer { size_t stat_module_cow_bytes; /* Copy on write bytes during module fork. */ uint64_t stat_clients_type_memory[CLIENT_TYPE_COUNT];/* Mem usage by type */ long long stat_unexpected_error_replies; /* Number of unexpected (aof-loading, replica to master, etc.) error replies */ + long long stat_dump_payload_sanitizations; /* Number deep dump payloads integrity validations. */ long long stat_io_reads_processed; /* Number of read events processed by IO / Main threads */ long long stat_io_writes_processed; /* Number of write events processed by IO / Main threads */ redisAtomic long long stat_total_reads_processed; /* Total number of read events processed */ @@ -1232,6 +1244,7 @@ struct redisServer { int active_expire_enabled; /* Can be disabled for testing purposes. */ int active_expire_effort; /* From 1 (default) to 10, active effort. */ int active_defrag_enabled; + int sanitize_dump_payload; /* Enables deep sanitization for ziplist and listpack in RDB and RESTORE. */ int jemalloc_bg_thread; /* Enable jemalloc background thread */ size_t active_defrag_ignore_bytes; /* minimum amount of fragmentation waste to start active defrag */ int active_defrag_threshold_lower; /* minimum percentage of fragmentation to start active defrag */ diff --git a/src/stream.h b/src/stream.h index 1a1e271a8..c7acee719 100644 --- a/src/stream.h +++ b/src/stream.h @@ -120,5 +120,6 @@ int streamIncrID(streamID *id); int streamDecrID(streamID *id); void streamPropagateConsumerCreation(client *c, robj *key, robj *groupname, sds consumername); robj *streamDup(robj *o); +int streamValidateListpackIntegrity(unsigned char *lp, size_t size, int deep); #endif diff --git a/src/t_hash.c b/src/t_hash.c index f3fde87aa..e6f14091b 100644 --- a/src/t_hash.c +++ b/src/t_hash.c @@ -67,7 +67,7 @@ int hashTypeGetFromZiplist(robj *o, sds field, zl = o->ptr; fptr = ziplistIndex(zl, ZIPLIST_HEAD); if (fptr != NULL) { - fptr = ziplistFind(fptr, (unsigned char*)field, sdslen(field), 1); + fptr = ziplistFind(zl, fptr, (unsigned char*)field, sdslen(field), 1); if (fptr != NULL) { /* Grab pointer to the value (fptr points to the field) */ vptr = ziplistNext(zl, fptr); @@ -208,7 +208,7 @@ int hashTypeSet(robj *o, sds field, sds value, int flags) { zl = o->ptr; fptr = ziplistIndex(zl, ZIPLIST_HEAD); if (fptr != NULL) { - fptr = ziplistFind(fptr, (unsigned char*)field, sdslen(field), 1); + fptr = ziplistFind(zl, fptr, (unsigned char*)field, sdslen(field), 1); if (fptr != NULL) { /* Grab pointer to the value (fptr points to the field) */ vptr = ziplistNext(zl, fptr); @@ -285,7 +285,7 @@ int hashTypeDelete(robj *o, sds field) { zl = o->ptr; fptr = ziplistIndex(zl, ZIPLIST_HEAD); if (fptr != NULL) { - fptr = ziplistFind(fptr, (unsigned char*)field, sdslen(field), 1); + fptr = ziplistFind(zl, fptr, (unsigned char*)field, sdslen(field), 1); if (fptr != NULL) { zl = ziplistDelete(zl,&fptr); /* Delete the key. */ zl = ziplistDelete(zl,&fptr); /* Delete the value. */ diff --git a/src/t_stream.c b/src/t_stream.c index bb38b0fb8..70cc8bfa1 100644 --- a/src/t_stream.c +++ b/src/t_stream.c @@ -255,21 +255,33 @@ unsigned char *lpReplaceInteger(unsigned char *lp, unsigned char **pos, int64_t /* This is a wrapper function for lpGet() to directly get an integer value * from the listpack (that may store numbers as a string), converting - * the string if needed. */ -int64_t lpGetInteger(unsigned char *ele) { + * the string if needed. + * The 'valid" argument is an optional output parameter to get an indication + * if the record was valid, when this parameter is NULL, the function will + * fail with an assertion. */ +static inline int64_t lpGetIntegerIfValid(unsigned char *ele, int *valid) { int64_t v; unsigned char *e = lpGet(ele,&v,NULL); - if (e == NULL) return v; + if (e == NULL) { + if (valid) + *valid = 1; + return v; + } /* The following code path should never be used for how listpacks work: * they should always be able to store an int64_t value in integer * encoded form. However the implementation may change. */ long long ll; - int retval = string2ll((char*)e,v,&ll); - serverAssert(retval != 0); + int ret = string2ll((char*)e,v,&ll); + if (valid) + *valid = ret; + else + serverAssert(ret != 0); v = ll; return v; } +#define lpGetInteger(ele) lpGetIntegerIfValid(ele, NULL) + /* Debugging function to log the full content of a listpack. Useful * for development and debugging. */ void streamLogListpackContent(unsigned char *lp) { @@ -3037,3 +3049,90 @@ NULL addReplySubcommandSyntaxError(c); } } + +/* Validate the integrity stream listpack entries stracture. Both in term of a + * valid listpack, but also that the stracture of the entires matches a valid + * stream. return 1 if valid 0 if not valid. */ +int streamValidateListpackIntegrity(unsigned char *lp, size_t size, int deep) { + int valid_record; + unsigned char *p, *next; + + /* Since we don't want to run validation of all records twice, we'll + * run the listpack validation of just the header and do the rest here. */ + if (!lpValidateIntegrity(lp, size, 0)) + return 0; + + /* In non-deep mode we just validated the listpack header (encoded size) */ + if (!deep) return 1; + + next = p = lpFirst(lp); + if (!lpValidateNext(lp, &next, size)) return 0; + if (!p) return 0; + + /* entry count */ + int64_t entry_count = lpGetIntegerIfValid(p, &valid_record); + if (!valid_record) return 0; + p = next; if (!lpValidateNext(lp, &next, size)) return 0; + + /* deleted */ + lpGetIntegerIfValid(p, &valid_record); + if (!valid_record) return 0; + p = next; if (!lpValidateNext(lp, &next, size)) return 0; + + /* num-of-fields */ + int64_t master_fields = lpGetIntegerIfValid(p, &valid_record); + if (!valid_record) return 0; + p = next; if (!lpValidateNext(lp, &next, size)) return 0; + + /* the field names */ + for (int64_t j = 0; j < master_fields; j++) { + p = next; if (!lpValidateNext(lp, &next, size)) return 0; + } + + /* the zero master entry terminator. */ + int64_t zero = lpGetIntegerIfValid(p, &valid_record); + if (!valid_record || zero != 0) return 0; + p = next; if (!lpValidateNext(lp, &next, size)) return 0; + + while (entry_count--) { + if (!p) return 0; + int64_t fields = master_fields, extra_fields = 3; + int64_t flags = lpGetIntegerIfValid(p, &valid_record); + if (!valid_record) return 0; + p = next; if (!lpValidateNext(lp, &next, size)) return 0; + + /* entry id */ + p = next; if (!lpValidateNext(lp, &next, size)) return 0; + p = next; if (!lpValidateNext(lp, &next, size)) return 0; + + if (!(flags & STREAM_ITEM_FLAG_SAMEFIELDS)) { + /* num-of-fields */ + fields = lpGetIntegerIfValid(p, &valid_record); + if (!valid_record) return 0; + p = next; if (!lpValidateNext(lp, &next, size)) return 0; + + /* the field names */ + for (int64_t j = 0; j < fields; j++) { + p = next; if (!lpValidateNext(lp, &next, size)) return 0; + } + + extra_fields += fields + 1; + } + + /* the values */ + for (int64_t j = 0; j < fields; j++) { + p = next; if (!lpValidateNext(lp, &next, size)) return 0; + } + + /* lp-count */ + int64_t lp_count = lpGetIntegerIfValid(p, &valid_record); + if (!valid_record) return 0; + if (lp_count != fields + extra_fields) return 0; + p = next; if (!lpValidateNext(lp, &next, size)) return 0; + } + + if (next) + return 0; + + return 1; +} diff --git a/src/ziplist.c b/src/ziplist.c index dd7452006..a8ab57cfe 100644 --- a/src/ziplist.c +++ b/src/ziplist.c @@ -152,6 +152,7 @@ * * Copyright (c) 2009-2012, Pieter Noordhuis * Copyright (c) 2009-2017, Salvatore Sanfilippo + * Copyright (c) 2020, Redis Labs, Inc * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -298,10 +299,34 @@ typedef struct zlentry { /* Extract the encoding from the byte pointed by 'ptr' and set it into * 'encoding' field of the zlentry structure. */ #define ZIP_ENTRY_ENCODING(ptr, encoding) do { \ - (encoding) = (ptr[0]); \ + (encoding) = ((ptr)[0]); \ if ((encoding) < ZIP_STR_MASK) (encoding) &= ZIP_STR_MASK; \ } while(0) +#define ZIP_ENCODING_SIZE_INVALID 0xff +/* Return the number of bytes required to encode the entry type + length. + * On error, return ZIP_ENCODING_SIZE_INVALID */ +static inline unsigned int zipEncodingLenSize(unsigned char encoding) { + if (encoding == ZIP_INT_16B || encoding == ZIP_INT_32B || + encoding == ZIP_INT_24B || encoding == ZIP_INT_64B || + encoding == ZIP_INT_8B) + return 1; + if (encoding >= ZIP_INT_IMM_MIN && encoding <= ZIP_INT_IMM_MAX) + return 1; + if (encoding == ZIP_STR_06B) + return 1; + if (encoding == ZIP_STR_14B) + return 2; + if (encoding == ZIP_STR_32B) + return 5; + return ZIP_ENCODING_SIZE_INVALID; +} + +#define ZIP_ASSERT_ENCODING(encoding) do { \ + if (zipEncodingLenSize(encoding) == ZIP_ENCODING_SIZE_INVALID) \ + panic("Invalid encoding 0x%02X", encoding); \ +} while (0) + /* Return bytes needed to store integer encoded by 'encoding'. */ unsigned int zipIntSize(unsigned char encoding) { switch(encoding) { @@ -313,7 +338,7 @@ unsigned int zipIntSize(unsigned char encoding) { } if (encoding >= ZIP_INT_IMM_MIN && encoding <= ZIP_INT_IMM_MAX) return 0; /* 4 bit immediate */ - panic("Invalid integer encoding 0x%02X", encoding); + /* bad encoding, covered by a previous call to ZIP_ASSERT_ENCODING */ return 0; } @@ -365,11 +390,10 @@ unsigned int zipStoreEntryEncoding(unsigned char *p, unsigned char encoding, uns /* Decode the entry encoding type and data length (string length for strings, * number of bytes used for the integer for integer entries) encoded in 'ptr'. - * The 'encoding' variable will hold the entry encoding, the 'lensize' + * The 'encoding' variable is input, extracted by the caller, the 'lensize' * variable will hold the number of bytes required to encode the entry * length, and the 'len' variable will hold the entry length. */ #define ZIP_DECODE_LENGTH(ptr, encoding, lensize, len) do { \ - ZIP_ENTRY_ENCODING((ptr), (encoding)); \ if ((encoding) < ZIP_STR_MASK) { \ if ((encoding) == ZIP_STR_06B) { \ (lensize) = 1; \ @@ -384,7 +408,8 @@ unsigned int zipStoreEntryEncoding(unsigned char *p, unsigned char encoding, uns ((ptr)[3] << 8) | \ ((ptr)[4]); \ } else { \ - panic("Invalid string encoding 0x%02X", (encoding)); \ + len = 0; /* bad encoding, dead code */ \ + /* covered by a previous call to ZIP_ASSERT_ENCODING */ \ } \ } else { \ (lensize) = 1; \ @@ -440,9 +465,10 @@ unsigned int zipStorePrevEntryLength(unsigned char *p, unsigned int len) { if ((prevlensize) == 1) { \ (prevlen) = (ptr)[0]; \ } else if ((prevlensize) == 5) { \ - assert(sizeof((prevlen)) == 4); \ - memcpy(&(prevlen), ((char*)(ptr)) + 1, 4); \ - memrev32ifbe(&prevlen); \ + (prevlen) = ((ptr)[4] << 24) | \ + ((ptr)[3] << 16) | \ + ((ptr)[2] << 8) | \ + ((ptr)[1]); \ } \ } while(0) @@ -467,14 +493,6 @@ int zipPrevLenByteDiff(unsigned char *p, unsigned int len) { return zipStorePrevEntryLength(NULL, len) - prevlensize; } -/* Return the total number of bytes used by the entry pointed to by 'p'. */ -unsigned int zipRawEntryLength(unsigned char *p) { - unsigned int prevlensize, encoding, lensize, len; - ZIP_DECODE_PREVLENSIZE(p, prevlensize); - ZIP_DECODE_LENGTH(p + prevlensize, encoding, lensize, len); - return prevlensize + lensize + len; -} - /* Check if string pointed to by 'entry' can be encoded as an integer. * Stores the integer value in 'v' and its encoding in 'encoding'. */ int zipTryEncoding(unsigned char *entry, unsigned int entrylen, long long *v, unsigned char *encoding) { @@ -565,15 +583,87 @@ int64_t zipLoadInteger(unsigned char *p, unsigned char encoding) { return ret; } -/* Return a struct with all information about an entry. */ +/* Fills a struct with all information about an entry. + * This function is the "unsafe" alternative to the one blow. + * Generally, all function that return a pointer to an element in the ziplist + * will assert that this element is valid, so it can be freely used. + * Generally functions such ziplistGet assume the input pointer is already + * validated (since it's the return value of another function). */ void zipEntry(unsigned char *p, zlentry *e) { - ZIP_DECODE_PREVLEN(p, e->prevrawlensize, e->prevrawlen); + ZIP_ENTRY_ENCODING(p + e->prevrawlensize, e->encoding); + ZIP_ASSERT_ENCODING(e->encoding); ZIP_DECODE_LENGTH(p + e->prevrawlensize, e->encoding, e->lensize, e->len); e->headersize = e->prevrawlensize + e->lensize; e->p = p; } +/* Fills a struct with all information about an entry. + * This function is safe to use on untrusted pointers, it'll make sure not to + * try to access memory outside the ziplist payload. + * Returns 1 if the entry is valid, and 0 otherwise. */ +static inline int zipEntrySafe(unsigned char* zl, size_t zlbytes, unsigned char *p, zlentry *e, int validate_prevlen) { +#define OUT_OF_RANGE(p) ( \ + (p) < zl + ZIPLIST_HEADER_SIZE || \ + (p) > zl + zlbytes - ZIPLIST_END_SIZE) + + /* Make sure the pointer doesn't rech outside the edge of the ziplist */ + if (OUT_OF_RANGE(p)) + return 0; + + /* Make sure the encoded prevlen header doesn't reach outside the allocation */ + ZIP_DECODE_PREVLENSIZE(p, e->prevrawlensize); + if (OUT_OF_RANGE(p + e->prevrawlensize)) + return 0; + + /* Make sure encoded entry header is valid. */ + ZIP_ENTRY_ENCODING(p + e->prevrawlensize, e->encoding); + e->lensize = zipEncodingLenSize(e->encoding); + if (e->lensize == ZIP_ENCODING_SIZE_INVALID) + return 0; + + /* Make sure the encoded entry header doesn't reach outside the allocation */ + if (OUT_OF_RANGE(p + e->prevrawlensize + e->lensize)) + return 0; + + /* Decode the prevlen and entry len headers. */ + ZIP_DECODE_PREVLEN(p, e->prevrawlensize, e->prevrawlen); + ZIP_DECODE_LENGTH(p + e->prevrawlensize, e->encoding, e->lensize, e->len); + e->headersize = e->prevrawlensize + e->lensize; + + /* Make sure the entry doesn't rech outside the edge of the ziplist */ + if (OUT_OF_RANGE(p + e->headersize + e->len)) + return 0; + + /* Make sure prevlen doesn't rech outside the edge of the ziplist */ + if (validate_prevlen && OUT_OF_RANGE(p - e->prevrawlen)) + return 0; + + e->p = p; + return 1; +#undef OUT_OF_RANGE +} + +/* Return the total number of bytes used by the entry pointed to by 'p'. */ +static inline unsigned int zipRawEntryLengthSafe(unsigned char* zl, size_t zlbytes, unsigned char *p) { + zlentry e; + assert(zipEntrySafe(zl, zlbytes, p, &e, 0)); + return e.headersize + e.len; +} + +/* Return the total number of bytes used by the entry pointed to by 'p'. */ +static inline unsigned int zipRawEntryLength(unsigned char *p) { + zlentry e; + zipEntry(p, &e); + return e.headersize + e.len; +} + +/* Validate that the entry doesn't reach outside the ziplist allocation. */ +static inline void zipAssertValidEntry(unsigned char* zl, size_t zlbytes, unsigned char *p) { + zlentry e; + assert(zipEntrySafe(zl, zlbytes, p, &e, 1)); +} + /* Create a new empty ziplist. */ unsigned char *ziplistNew(void) { unsigned int bytes = ZIPLIST_HEADER_SIZE+ZIPLIST_END_SIZE; @@ -625,7 +715,7 @@ unsigned char *__ziplistCascadeUpdate(unsigned char *zl, unsigned char *p) { /* Empty ziplist */ if (p[0] == ZIP_END) return zl; - zipEntry(p, &cur); + zipEntry(p, &cur); /* no need for "safe" variant since the input pointer was validated by the function that returned it. */ firstentrylen = prevlen = cur.headersize + cur.len; prevlensize = zipStorePrevEntryLength(NULL, prevlen); prevoffset = p - zl; @@ -633,7 +723,7 @@ unsigned char *__ziplistCascadeUpdate(unsigned char *zl, unsigned char *p) { /* Iterate ziplist to find out how many extra bytes do we need to update it. */ while (p[0] != ZIP_END) { - zipEntry(p, &cur); + assert(zipEntrySafe(zl, curlen, p, &cur, 0)); /* Abort when "prevlen" has not changed. */ if (cur.prevrawlen == prevlen) break; @@ -690,7 +780,7 @@ unsigned char *__ziplistCascadeUpdate(unsigned char *zl, unsigned char *p) { /* Iterate all entries that need to be updated tail to head. */ while (cnt) { - zipEntry(zl + prevoffset, &cur); + zipEntry(zl + prevoffset, &cur); /* no need for "safe" variant since we already iterated on all these entries above. */ rawlen = cur.headersize + cur.len; /* Move entry to tail and reset prevlen. */ memmove(p - (rawlen - cur.prevrawlensize), @@ -717,15 +807,18 @@ unsigned char *__ziplistDelete(unsigned char *zl, unsigned char *p, unsigned int size_t offset; int nextdiff = 0; zlentry first, tail; + size_t zlbytes = intrev32ifbe(ZIPLIST_BYTES(zl)); - zipEntry(p, &first); + zipEntry(p, &first); /* no need for "safe" variant since the input pointer was validated by the function that returned it. */ for (i = 0; p[0] != ZIP_END && i < num; i++) { - p += zipRawEntryLength(p); + p += zipRawEntryLengthSafe(zl, zlbytes, p); deleted++; } + assert(p >= first.p); totlen = p-first.p; /* Bytes taken by the element(s) to delete. */ if (totlen > 0) { + uint32_t set_tail; if (p[0] != ZIP_END) { /* Storing `prevrawlen` in this entry may increase or decrease the * number of bytes required compare to the current `prevrawlen`. @@ -738,36 +831,44 @@ unsigned char *__ziplistDelete(unsigned char *zl, unsigned char *p, unsigned int * had a 5 bytes prevlen header, so there is for sure at least * 5 bytes free and we need just 4. */ p -= nextdiff; + assert(p >= first.p && p= first.p. we know totlen >= 0, + * so we know that p > first.p and this is guaranteed not to reach + * beyond the allocation, even if the entries lens are corrupted. */ + size_t bytes_to_move = zlbytes-(p-zl)-1; + memmove(first.p,p,bytes_to_move); } else { /* The entire tail was deleted. No need to move memory. */ - ZIPLIST_TAIL_OFFSET(zl) = - intrev32ifbe((first.p-zl)-first.prevrawlen); + set_tail = (first.p-zl)-first.prevrawlen; } - /* Resize and update length */ + /* Resize the ziplist */ offset = first.p-zl; - zl = ziplistResize(zl, intrev32ifbe(ZIPLIST_BYTES(zl))-totlen+nextdiff); - ZIPLIST_INCR_LENGTH(zl,-deleted); + zlbytes -= totlen - nextdiff; + zl = ziplistResize(zl, zlbytes); p = zl+offset; + /* Update record count */ + ZIPLIST_INCR_LENGTH(zl,-deleted); + + /* Set the tail offset computed above */ + assert(set_tail <= zlbytes - ZIPLIST_END_SIZE); + ZIPLIST_TAIL_OFFSET(zl) = intrev32ifbe(set_tail); + /* When nextdiff != 0, the raw length of the next entry has changed, so * we need to cascade the update throughout the ziplist */ if (nextdiff != 0) @@ -778,7 +879,7 @@ unsigned char *__ziplistDelete(unsigned char *zl, unsigned char *p, unsigned int /* Insert item at "p". */ unsigned char *__ziplistInsert(unsigned char *zl, unsigned char *p, unsigned char *s, unsigned int slen) { - size_t curlen = intrev32ifbe(ZIPLIST_BYTES(zl)), reqlen; + size_t curlen = intrev32ifbe(ZIPLIST_BYTES(zl)), reqlen, newlen; unsigned int prevlensize, prevlen = 0; size_t offset; int nextdiff = 0; @@ -794,7 +895,7 @@ unsigned char *__ziplistInsert(unsigned char *zl, unsigned char *p, unsigned cha } else { unsigned char *ptail = ZIPLIST_ENTRY_TAIL(zl); if (ptail[0] != ZIP_END) { - prevlen = zipRawEntryLength(ptail); + prevlen = zipRawEntryLengthSafe(zl, curlen, ptail); } } @@ -824,7 +925,8 @@ unsigned char *__ziplistInsert(unsigned char *zl, unsigned char *p, unsigned cha /* Store offset because a realloc may change the address of zl. */ offset = p-zl; - zl = ziplistResize(zl,curlen+reqlen+nextdiff); + newlen = curlen+reqlen+nextdiff; + zl = ziplistResize(zl,newlen); p = zl+offset; /* Apply memory move when necessary and update tail offset. */ @@ -845,7 +947,7 @@ unsigned char *__ziplistInsert(unsigned char *zl, unsigned char *p, unsigned cha /* When the tail contains more than one entry, we need to take * "nextdiff" in account as well. Otherwise, a change in the * size of prevlen doesn't have an effect on the *tail* offset. */ - zipEntry(p+reqlen, &tail); + assert(zipEntrySafe(zl, newlen, p+reqlen, &tail, 1)); if (p[reqlen+tail.headersize+tail.len] != ZIP_END) { ZIPLIST_TAIL_OFFSET(zl) = intrev32ifbe(intrev32ifbe(ZIPLIST_TAIL_OFFSET(zl))+nextdiff); @@ -1002,23 +1104,35 @@ unsigned char *ziplistPush(unsigned char *zl, unsigned char *s, unsigned int sle unsigned char *ziplistIndex(unsigned char *zl, int index) { unsigned char *p; unsigned int prevlensize, prevlen = 0; + size_t zlbytes = intrev32ifbe(ZIPLIST_BYTES(zl)); if (index < 0) { index = (-index)-1; p = ZIPLIST_ENTRY_TAIL(zl); if (p[0] != ZIP_END) { + /* No need for "safe" check: when going backwards, we know the header + * we're parsing is in the range, we just need to assert (below) that + * the size we take doesn't cause p to go outside the allocation. */ ZIP_DECODE_PREVLEN(p, prevlensize, prevlen); while (prevlen > 0 && index--) { p -= prevlen; + assert(p >= zl + ZIPLIST_HEADER_SIZE && p < zl + zlbytes - ZIPLIST_END_SIZE); ZIP_DECODE_PREVLEN(p, prevlensize, prevlen); } } } else { p = ZIPLIST_ENTRY_HEAD(zl); - while (p[0] != ZIP_END && index--) { - p += zipRawEntryLength(p); + while (index--) { + /* Use the "safe" length: When we go forward, we need to be careful + * not to decode an entry header if it's past the ziplist allocation. */ + p += zipRawEntryLengthSafe(zl, zlbytes, p); + if (p[0] == ZIP_END) + break; } } - return (p[0] == ZIP_END || index > 0) ? NULL : p; + if (p[0] == ZIP_END || index > 0) + return NULL; + zipAssertValidEntry(zl, zlbytes, p); + return p; } /* Return pointer to next entry in ziplist. @@ -1029,6 +1143,7 @@ unsigned char *ziplistIndex(unsigned char *zl, int index) { * The element after 'p' is returned, otherwise NULL if we are at the end. */ unsigned char *ziplistNext(unsigned char *zl, unsigned char *p) { ((void) zl); + size_t zlbytes = intrev32ifbe(ZIPLIST_BYTES(zl)); /* "p" could be equal to ZIP_END, caused by ziplistDelete, * and we should return NULL. Otherwise, we should return NULL @@ -1042,6 +1157,7 @@ unsigned char *ziplistNext(unsigned char *zl, unsigned char *p) { return NULL; } + zipAssertValidEntry(zl, zlbytes, p); return p; } @@ -1060,7 +1176,10 @@ unsigned char *ziplistPrev(unsigned char *zl, unsigned char *p) { } else { ZIP_DECODE_PREVLEN(p, prevlensize, prevlen); assert(prevlen > 0); - return p-prevlen; + p-=prevlen; + size_t zlbytes = intrev32ifbe(ZIPLIST_BYTES(zl)); + zipAssertValidEntry(zl, zlbytes, p); + return p; } } @@ -1073,7 +1192,7 @@ unsigned int ziplistGet(unsigned char *p, unsigned char **sstr, unsigned int *sl if (p == NULL || p[0] == ZIP_END) return 0; if (sstr) *sstr = NULL; - zipEntry(p, &entry); + zipEntry(p, &entry); /* no need for "safe" variant since the input pointer was validated by the function that returned it. */ if (ZIP_IS_STR(entry.encoding)) { if (sstr) { *slen = entry.len; @@ -1121,7 +1240,7 @@ unsigned int ziplistCompare(unsigned char *p, unsigned char *sstr, unsigned int long long zval, sval; if (p[0] == ZIP_END) return 0; - zipEntry(p, &entry); + zipEntry(p, &entry); /* no need for "safe" variant since the input pointer was validated by the function that returned it. */ if (ZIP_IS_STR(entry.encoding)) { /* Raw compare */ if (entry.len == slen) { @@ -1142,23 +1261,23 @@ unsigned int ziplistCompare(unsigned char *p, unsigned char *sstr, unsigned int /* Find pointer to the entry equal to the specified entry. Skip 'skip' entries * between every comparison. Returns NULL when the field could not be found. */ -unsigned char *ziplistFind(unsigned char *p, unsigned char *vstr, unsigned int vlen, unsigned int skip) { +unsigned char *ziplistFind(unsigned char *zl, unsigned char *p, unsigned char *vstr, unsigned int vlen, unsigned int skip) { int skipcnt = 0; unsigned char vencoding = 0; long long vll = 0; + size_t zlbytes = ziplistBlobLen(zl); while (p[0] != ZIP_END) { - unsigned int prevlensize, encoding, lensize, len; + struct zlentry e; unsigned char *q; - ZIP_DECODE_PREVLENSIZE(p, prevlensize); - ZIP_DECODE_LENGTH(p + prevlensize, encoding, lensize, len); - q = p + prevlensize + lensize; + assert(zipEntrySafe(zl, zlbytes, p, &e, 1)); + q = p + e.prevrawlensize + e.lensize; if (skipcnt == 0) { /* Compare current entry with specified entry */ - if (ZIP_IS_STR(encoding)) { - if (len == vlen && memcmp(q, vstr, vlen) == 0) { + if (ZIP_IS_STR(e.encoding)) { + if (e.len == vlen && memcmp(q, vstr, vlen) == 0) { return p; } } else { @@ -1180,7 +1299,7 @@ unsigned char *ziplistFind(unsigned char *p, unsigned char *vstr, unsigned int v * if vencoding != UCHAR_MAX because if there is no encoding * possible for the field it can't be a valid integer. */ if (vencoding != UCHAR_MAX) { - long long ll = zipLoadInteger(q, encoding); + long long ll = zipLoadInteger(q, e.encoding); if (ll == vll) { return p; } @@ -1195,7 +1314,7 @@ unsigned char *ziplistFind(unsigned char *p, unsigned char *vstr, unsigned int v } /* Move to next entry */ - p = q + len; + p = q + e.len; } return NULL; @@ -1208,8 +1327,9 @@ unsigned int ziplistLen(unsigned char *zl) { len = intrev16ifbe(ZIPLIST_LENGTH(zl)); } else { unsigned char *p = zl+ZIPLIST_HEADER_SIZE; + size_t zlbytes = intrev32ifbe(ZIPLIST_BYTES(zl)); while (*p != ZIP_END) { - p += zipRawEntryLength(p); + p += zipRawEntryLengthSafe(zl, zlbytes, p); len++; } @@ -1228,6 +1348,7 @@ void ziplistRepr(unsigned char *zl) { unsigned char *p; int index = 0; zlentry entry; + size_t zlbytes = ziplistBlobLen(zl); printf( "{total bytes %u} " @@ -1238,7 +1359,7 @@ void ziplistRepr(unsigned char *zl) { intrev32ifbe(ZIPLIST_TAIL_OFFSET(zl))); p = ZIPLIST_ENTRY_HEAD(zl); while(*p != ZIP_END) { - zipEntry(p, &entry); + assert(zipEntrySafe(zl, zlbytes, p, &entry, 1)); printf( "{\n" "\taddr 0x%08lx,\n" @@ -1282,6 +1403,63 @@ void ziplistRepr(unsigned char *zl) { printf("{end}\n\n"); } +/* Validate the integrity of the data stracture. + * when `deep` is 0, only the integrity of the header is validated. + * when `deep` is 1, we scan all the entries one by one. */ +int ziplistValidateIntegrity(unsigned char *zl, size_t size, int deep) { + /* check that we can actually read the header. (and ZIP_END) */ + if (size < ZIPLIST_HEADER_SIZE + ZIPLIST_END_SIZE) + return 0; + + /* check that the encoded size in the header must match the allocated size. */ + size_t bytes = intrev32ifbe(ZIPLIST_BYTES(zl)); + if (bytes != size) + return 0; + + /* the last byte must be the terminator. */ + if (zl[size - ZIPLIST_END_SIZE] != ZIP_END) + return 0; + + /* make sure the tail offset isn't reaching outside the allocation. */ + if (intrev32ifbe(ZIPLIST_TAIL_OFFSET(zl)) > size - ZIPLIST_END_SIZE) + return 0; + + if (!deep) + return 1; + + unsigned int count = 0; + unsigned char *p = ZIPLIST_ENTRY_HEAD(zl); + unsigned char *prev = NULL; + size_t prev_raw_size = 0; + while(*p != ZIP_END) { + struct zlentry e; + /* Decode the entry headers and fail if invalid or reaches outside the allocation */ + if (!zipEntrySafe(zl, size, p, &e, 1)) + return 0; + + /* Make sure the record stating the prev entry size is correct. */ + if (e.prevrawlen != prev_raw_size) + return 0; + + /* Move to the next entry */ + prev_raw_size = e.headersize + e.len; + prev = p; + p += e.headersize + e.len; + count++; + } + + /* Make sure the entry really do point to the start of the last entry. */ + if (prev != ZIPLIST_ENTRY_TAIL(zl)) + return 0; + + /* Check that the count in the header is correct */ + unsigned int header_count = intrev16ifbe(ZIPLIST_LENGTH(zl)); + if (header_count != UINT16_MAX && count != header_count) + return 0; + + return 1; +} + #ifdef REDIS_TEST #include #include "adlist.h" @@ -1784,6 +1962,7 @@ int ziplistTest(int argc, char **argv) { printf("Create long list and check indices:\n"); { + unsigned long long start = usec(); zl = ziplistNew(); char buf[32]; int i,len; @@ -1800,7 +1979,7 @@ int ziplistTest(int argc, char **argv) { assert(ziplistGet(p,NULL,NULL,&value)); assert(999-i == value); } - printf("SUCCESS\n\n"); + printf("SUCCESS. usec=%lld\n\n", usec()-start); zfree(zl); } @@ -1907,6 +2086,7 @@ int ziplistTest(int argc, char **argv) { printf("Stress with random payloads of different encoding:\n"); { + unsigned long long start = usec(); int i,j,len,where; unsigned char *p; char buf[1024]; @@ -1979,13 +2159,15 @@ int ziplistTest(int argc, char **argv) { zfree(zl); listRelease(ref); } - printf("SUCCESS\n\n"); + printf("Done. usec=%lld\n\n", usec()-start); } printf("Stress with variable ziplist size:\n"); { + unsigned long long start = usec(); stress(ZIPLIST_HEAD,100000,16384,256); stress(ZIPLIST_TAIL,100000,16384,256); + printf("Done. usec=%lld\n\n", usec()-start); } printf("Stress __ziplistCascadeUpdate:\n"); diff --git a/src/ziplist.h b/src/ziplist.h index 964a47f6d..8d1ac1691 100644 --- a/src/ziplist.h +++ b/src/ziplist.h @@ -45,10 +45,11 @@ unsigned char *ziplistInsert(unsigned char *zl, unsigned char *p, unsigned char unsigned char *ziplistDelete(unsigned char *zl, unsigned char **p); unsigned char *ziplistDeleteRange(unsigned char *zl, int index, unsigned int num); unsigned int ziplistCompare(unsigned char *p, unsigned char *s, unsigned int slen); -unsigned char *ziplistFind(unsigned char *p, unsigned char *vstr, unsigned int vlen, unsigned int skip); +unsigned char *ziplistFind(unsigned char *zl, unsigned char *p, unsigned char *vstr, unsigned int vlen, unsigned int skip); unsigned int ziplistLen(unsigned char *zl); size_t ziplistBlobLen(unsigned char *zl); void ziplistRepr(unsigned char *zl); +int ziplistValidateIntegrity(unsigned char *zl, size_t size, int deep); #ifdef REDIS_TEST int ziplistTest(int argc, char *argv[]); diff --git a/src/zipmap.c b/src/zipmap.c index 365c4aea4..bd41fe6a5 100644 --- a/src/zipmap.c +++ b/src/zipmap.c @@ -111,6 +111,10 @@ static unsigned int zipmapDecodeLength(unsigned char *p) { return len; } +static unsigned int zipmapGetEncodedLengthSize(unsigned char *p) { + return (*p < ZIPMAP_BIGLEN) ? 1: 5; +} + /* Encode the length 'l' writing it in 'p'. If p is NULL it just returns * the amount of bytes required to encode such a length. */ static unsigned int zipmapEncodeLength(unsigned char *p, unsigned int len) { @@ -370,6 +374,70 @@ size_t zipmapBlobLen(unsigned char *zm) { return totlen; } +/* Validate the integrity of the data stracture. + * when `deep` is 0, only the integrity of the header is validated. + * when `deep` is 1, we scan all the entries one by one. */ +int zipmapValidateIntegrity(unsigned char *zm, size_t size, int deep) { +#define OUT_OF_RANGE(p) ( \ + (p) < zm + 2 || \ + (p) > zm + size - 1) + unsigned int l, s, e; + + /* check that we can actually read the header (or ZIPMAP_END). */ + if (size < 2) + return 0; + + /* the last byte must be the terminator. */ + if (zm[size-1] != ZIPMAP_END) + return 0; + + if (!deep) + return 1; + + unsigned int count = 0; + unsigned char *p = zm + 1; /* skip the count */ + while(*p != ZIPMAP_END) { + /* read the field name length encoding type */ + s = zipmapGetEncodedLengthSize(p); + /* make sure the entry length doesn't rech outside the edge of the zipmap */ + if (OUT_OF_RANGE(p+s)) + return 0; + + /* read the field name length */ + l = zipmapDecodeLength(p); + p += s; /* skip the encoded field size */ + p += l; /* skip the field */ + + /* make sure the entry doesn't rech outside the edge of the zipmap */ + if (OUT_OF_RANGE(p)) + return 0; + + /* read the value length encoding type */ + s = zipmapGetEncodedLengthSize(p); + /* make sure the entry length doesn't rech outside the edge of the zipmap */ + if (OUT_OF_RANGE(p+s)) + return 0; + + /* read the value length */ + l = zipmapDecodeLength(p); + p += s; /* skip the encoded value size*/ + e = *p++; /* skip the encoded free space (always encoded in one byte) */ + p += l+e; /* skip the value and free space */ + count++; + + /* make sure the entry doesn't rech outside the edge of the zipmap */ + if (OUT_OF_RANGE(p)) + return 0; + } + + /* check that the count in the header is correct */ + if (zm[0] != ZIPMAP_BIGLEN && zm[0] != count) + return 0; + + return 1; +#undef OUT_OF_RANGE +} + #ifdef REDIS_TEST static void zipmapRepr(unsigned char *p) { unsigned int l; diff --git a/src/zipmap.h b/src/zipmap.h index ac588f05a..daf8430a0 100644 --- a/src/zipmap.h +++ b/src/zipmap.h @@ -45,6 +45,7 @@ int zipmapExists(unsigned char *zm, unsigned char *key, unsigned int klen); unsigned int zipmapLen(unsigned char *zm); size_t zipmapBlobLen(unsigned char *zm); void zipmapRepr(unsigned char *p); +int zipmapValidateIntegrity(unsigned char *zm, size_t size, int deep); #ifdef REDIS_TEST int zipmapTest(int argc, char *argv[]); diff --git a/tests/assets/corrupt_ziplist.rdb b/tests/assets/corrupt_ziplist.rdb new file mode 100644 index 000000000..b40ada8d6 Binary files /dev/null and b/tests/assets/corrupt_ziplist.rdb differ diff --git a/tests/cluster/tests/04-resharding.tcl b/tests/cluster/tests/04-resharding.tcl index cee2ec5ba..1dcdb5a2c 100644 --- a/tests/cluster/tests/04-resharding.tcl +++ b/tests/cluster/tests/04-resharding.tcl @@ -172,3 +172,10 @@ test "Verify slaves consistency" { } assert {$verified_masters >= 5} } + +test "Dump sanitization was skipped for migrations" { + set verified_masters 0 + foreach_redis_id id { + assert {[RI $id dump_payload_sanitizations] == 0} + } +} diff --git a/tests/integration/corrupt-dump.tcl b/tests/integration/corrupt-dump.tcl new file mode 100644 index 000000000..9864036d2 --- /dev/null +++ b/tests/integration/corrupt-dump.tcl @@ -0,0 +1,178 @@ +# tests of corrupt ziplist payload with valid CRC + +tags {"dump" "corruption"} { + +set corrupt_payload_7445 "\x0E\x01\x1D\x1D\x00\x00\x00\x16\x00\x00\x00\x03\x00\x00\x04\x43\x43\x43\x43\x06\x04\x42\x42\x42\x42\x06\x3F\x41\x41\x41\x41\xFF\x09\x00\x88\xA5\xCA\xA8\xC5\x41\xF4\x35" + +test {corrupt payload: #7445 - with sanitize} { + start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { + r config set sanitize-dump-payload yes + catch { + r restore key 0 $corrupt_payload_7445 + } err + assert_match "*Bad data format*" $err + verify_log_message 0 "*Ziplist integrity check failed*" 0 + } +} + +test {corrupt payload: #7445 - without sanitize - 1} { + start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { + r config set sanitize-dump-payload no + r config set crash-memcheck-enabled no ;# avoid valgrind issues + r restore key 0 $corrupt_payload_7445 + catch {r lindex key 2} + verify_log_message 0 "*ASSERTION FAILED*" 0 + } +} + +test {corrupt payload: #7445 - without sanitize - 2} { + start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { + r config set sanitize-dump-payload no + r config set crash-memcheck-enabled no ;# avoid valgrind issues + r restore key 0 $corrupt_payload_7445 + catch {r lset key 2 "BEEF"} + verify_log_message 0 "*ASSERTION FAILED*" 0 + } +} + +test {corrupt payload: hash with valid zip list header, invalid entry len} { + start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { + r restore key 0 "\x0D\x1B\x1B\x00\x00\x00\x16\x00\x00\x00\x04\x00\x00\x02\x61\x00\x04\x02\x62\x00\x04\x14\x63\x00\x04\x02\x64\x00\xFF\x09\x00\xD9\x10\x54\x92\x15\xF5\x5F\x52" + r config set crash-memcheck-enabled no ;# avoid valgrind issues + r config set hash-max-ziplist-entries 1 + catch {r hset key b b} + verify_log_message 0 "*zipEntrySafe*" 0 + } +} + +test {corrupt payload: invalid zlbytes header} { + start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { + catch { + r restore key 0 "\x0D\x1B\x25\x00\x00\x00\x16\x00\x00\x00\x04\x00\x00\x02\x61\x00\x04\x02\x62\x00\x04\x02\x63\x00\x04\x02\x64\x00\xFF\x09\x00\xB7\xF7\x6E\x9F\x43\x43\x14\xC6" + } err + assert_match "*Bad data format*" $err + } +} + +test {corrupt payload: valid zipped hash header, dup records} { + start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { + r restore key 0 "\x0D\x1B\x1B\x00\x00\x00\x16\x00\x00\x00\x04\x00\x00\x02\x61\x00\x04\x02\x62\x00\x04\x02\x61\x00\x04\x02\x64\x00\xFF\x09\x00\xA1\x98\x36\x78\xCC\x8E\x93\x2E" + r config set crash-memcheck-enabled no ;# avoid valgrind issues + r config set hash-max-ziplist-entries 1 + # cause an assertion when converting to hash table + catch {r hset key b b} + verify_log_message 0 "*ziplist with dup elements dump*" 0 + } +} + +test {corrupt payload: quicklist big ziplist prev len} { + start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { + r restore key 0 "\x0E\x01\x13\x13\x00\x00\x00\x0E\x00\x00\x00\x02\x00\x00\x02\x61\x00\x0E\x02\x62\x00\xFF\x09\x00\x49\x97\x30\xB2\x0D\xA1\xED\xAA" + r config set crash-memcheck-enabled no ;# avoid valgrind issues + catch {r lindex key -2} + verify_log_message 0 "*ASSERTION FAILED*" 0 + } +} + +test {corrupt payload: quicklist small ziplist prev len} { + start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { + r config set sanitize-dump-payload yes + catch { + r restore key 0 "\x0E\x01\x13\x13\x00\x00\x00\x0E\x00\x00\x00\x02\x00\x00\x02\x61\x00\x02\x02\x62\x00\xFF\x09\x00\xC7\x71\x03\x97\x07\x75\xB0\x63" + } err + assert_match "*Bad data format*" $err + verify_log_message 0 "*Ziplist integrity check failed*" 0 + } +} + +test {corrupt payload: quicklist ziplist wrong count} { + start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { + r restore key 0 "\x0E\x01\x13\x13\x00\x00\x00\x0E\x00\x00\x00\x03\x00\x00\x02\x61\x00\x04\x02\x62\x00\xFF\x09\x00\x4D\xE2\x0A\x2F\x08\x25\xDF\x91" + r lpush key a + # check that the server didn't crash + r ping + } +} + +test {corrupt payload: #3080 - quicklist} { + start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { + catch { + r RESTORE key 0 "\x0E\x01\x80\x00\x00\x00\x10\x41\x41\x41\x41\x41\x41\x41\x41\x02\x00\x00\x80\x41\x41\x41\x41\x07\x00\x03\xC7\x1D\xEF\x54\x68\xCC\xF3" + r DUMP key + } + assert_match "*Bad data format*" $err + verify_log_message 0 "*Ziplist integrity check failed*" 0 + } +} + +test {corrupt payload: #3080 - ziplist} { + start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { + catch { + r RESTORE key 0 "\x0A\x80\x00\x00\x00\x10\x41\x41\x41\x41\x41\x41\x41\x41\x02\x00\x00\x80\x41\x41\x41\x41\x07\x00\x39\x5B\x49\xE0\xC1\xC6\xDD\x76" + } + assert_match "*Bad data format*" $err + verify_log_message 0 "*Ziplist integrity check failed*" 0 + } +} + +test {corrupt payload: load corrupted rdb with no CRC - #3505} { + set server_path [tmpdir "server.rdb-corruption-test"] + exec cp tests/assets/corrupt_ziplist.rdb $server_path + set srv [start_server [list overrides [list "dir" $server_path "dbfilename" "corrupt_ziplist.rdb" loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no]]] + + # wait for termination + wait_for_condition 100 50 { + ! [is_alive $srv] + } else { + fail "rdb loading didn't fail" + } + + set stdout [dict get $srv stdout] + assert_equal [count_message_lines $stdout "Terminating server after rdb file reading failure."] 1 + assert_lessthan 1 [count_message_lines $stdout "Ziplist integrity check failed"] + kill_server $srv ;# let valgrind look for issues +} + +test {corrupt payload: listpack invalid size header} { + start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { + catch { + r restore key 0 "\x0F\x01\x10\x00\x00\x00\x00\x00\x00\x00\x02\x00\x00\x00\x00\x00\x00\x00\x02\x40\x55\x5F\x00\x00\x00\x0F\x00\x01\x01\x00\x01\x02\x01\x88\x31\x00\x00\x00\x00\x00\x00\x00\x09\x88\x32\x00\x00\x00\x00\x00\x00\x00\x09\x00\x01\x00\x01\x00\x01\x00\x01\x02\x02\x88\x31\x00\x00\x00\x00\x00\x00\x00\x09\x88\x61\x00\x00\x00\x00\x00\x00\x00\x09\x88\x32\x00\x00\x00\x00\x00\x00\x00\x09\x88\x62\x00\x00\x00\x00\x00\x00\x00\x09\x08\x01\xFF\x0A\x01\x00\x00\x09\x00\x45\x91\x0A\x87\x2F\xA5\xF9\x2E" + } err + assert_match "*Bad data format*" $err + verify_log_message 0 "*Stream listpack integrity check failed*" 0 + } +} + +test {corrupt payload: listpack too long entry len} { + start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { + r restore key 0 "\x0F\x01\x10\x00\x00\x00\x00\x00\x00\x00\x02\x00\x00\x00\x00\x00\x00\x00\x02\x40\x55\x55\x00\x00\x00\x0F\x00\x01\x01\x00\x01\x02\x01\x88\x31\x00\x00\x00\x00\x00\x00\x00\x09\x88\x32\x00\x00\x00\x00\x00\x00\x00\x09\x00\x01\x00\x01\x00\x01\x00\x01\x02\x02\x89\x31\x00\x00\x00\x00\x00\x00\x00\x09\x88\x61\x00\x00\x00\x00\x00\x00\x00\x09\x88\x32\x00\x00\x00\x00\x00\x00\x00\x09\x88\x62\x00\x00\x00\x00\x00\x00\x00\x09\x08\x01\xFF\x0A\x01\x00\x00\x09\x00\x40\x63\xC9\x37\x03\xA2\xE5\x68" + catch { + r xinfo stream key full + } err + verify_log_message 0 "*ASSERTION FAILED*" 0 + } +} + +test {corrupt payload: listpack very long entry len} { + start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { + r restore key 0 "\x0F\x01\x10\x00\x00\x00\x00\x00\x00\x00\x02\x00\x00\x00\x00\x00\x00\x00\x02\x40\x55\x55\x00\x00\x00\x0F\x00\x01\x01\x00\x01\x02\x01\x88\x31\x00\x00\x00\x00\x00\x00\x00\x09\x88\x32\x00\x00\x00\x00\x00\x00\x00\x09\x00\x01\x00\x01\x00\x01\x00\x01\x02\x02\x88\x31\x00\x00\x00\x00\x00\x00\x00\x09\x88\x61\x00\x00\x00\x00\x00\x00\x00\x09\x88\x32\x00\x00\x00\x00\x00\x00\x00\x09\x9C\x62\x00\x00\x00\x00\x00\x00\x00\x09\x08\x01\xFF\x0A\x01\x00\x00\x09\x00\x63\x6F\x42\x8E\x7C\xB5\xA2\x9D" + catch { + r xinfo stream key full + } err + verify_log_message 0 "*ASSERTION FAILED*" 0 + } +} + +test {corrupt payload: listpack too long entry prev len} { + start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { + r config set sanitize-dump-payload yes + catch { + r restore key 0 "\x0F\x01\x10\x00\x00\x00\x00\x00\x00\x00\x02\x00\x00\x00\x00\x00\x00\x00\x02\x40\x55\x55\x00\x00\x00\x0F\x00\x01\x01\x00\x15\x02\x01\x88\x31\x00\x00\x00\x00\x00\x00\x00\x09\x88\x32\x00\x00\x00\x00\x00\x00\x00\x09\x00\x01\x00\x01\x00\x01\x00\x01\x02\x02\x88\x31\x00\x00\x00\x00\x00\x00\x00\x09\x88\x61\x00\x00\x00\x00\x00\x00\x00\x09\x88\x32\x00\x00\x00\x00\x00\x00\x00\x09\x88\x62\x00\x00\x00\x00\x00\x00\x00\x09\x08\x01\xFF\x0A\x01\x00\x00\x09\x00\x06\xFB\x44\x24\x0A\x8E\x75\xEA" + } err + assert_match "*Bad data format*" $err + verify_log_message 0 "*Stream listpack integrity check failed*" 0 + } +} + +} ;# tags + diff --git a/tests/support/util.tcl b/tests/support/util.tcl index 3268bc974..aadec7cd1 100644 --- a/tests/support/util.tcl +++ b/tests/support/util.tcl @@ -112,6 +112,22 @@ proc count_log_lines {srv_idx} { set _ [exec wc -l < [srv $srv_idx stdout]] } +# returns the number of times a line with that pattern appears in a file +proc count_message_lines {file pattern} { + set res 0 + # exec fails when grep exists with status other than 0 (when the patter wasn't found) + catch { + set res [exec grep $pattern $file 2> /dev/null | wc -l] + } + return $res +} + +# returns the number of times a line with that pattern appears in the log +proc count_log_message {srv_idx pattern} { + set stdout [srv $srv_idx stdout] + return [count_message_lines $stdout $pattern] +} + # verify pattern exists in server's sdtout after a certain line number proc verify_log_message {srv_idx pattern from_line} { incr from_line diff --git a/tests/test_helper.tcl b/tests/test_helper.tcl index 6a38cc001..998e82bd9 100644 --- a/tests/test_helper.tcl +++ b/tests/test_helper.tcl @@ -44,6 +44,7 @@ set ::all_tests { integration/replication-psync integration/aof integration/rdb + integration/corrupt-dump integration/convert-zipmap-hash-on-load integration/logging integration/psync2