1. 27 Apr, 2022 14 commits
    • sundb's avatar
      Santize dump payload: fix invalid listpack entry start with EOF (#9889) · db3e3ebc
      sundb authored
      When an invalid listpack entry starts with EOF, we will skip it when we verify it in the loop.
      
      (cherry picked from commit 1808618f)
      db3e3ebc
    • OfirMos's avatar
      fixed mem leak on rdb load error (#9860) · 86db5091
      OfirMos authored
      a rare case of short read that can happen when breaking the master-replica
      connection on diskless load mode,
      
      (cherry picked from commit 9f9c7857)
      86db5091
    • Meir Shpilraien (Spielrein)'s avatar
      Clean Lua stack before parsing call reply to avoid crash on a call with many arguments (#9809) · 93c1d31d
      Meir Shpilraien (Spielrein) authored
      This commit 0f8b634c (CVE-2021-32626 released in 6.2.6, 6.0.16, 5.0.14)
      fixes an invalid memory write issue by using `lua_checkstack` API to make
      sure the Lua stack is not overflow. This fix was added on 3 places:
      1. `luaReplyToRedisReply`
      2. `ldbRedis`
      3. `redisProtocolToLuaType`
      
      On the first 2 functions, `lua_checkstack` is handled gracefully while the
      last is handled with an assert and a statement that this situation can
      not happened (only with misbehave module):
      
      > the Redis reply might be deep enough to explode the LUA stack (notice
      that currently there is no such command in Redis that returns such a nested
      reply, but modules might do it)
      
      The issue that was discovered is that user arguments is also considered part
      of the stack, and so the following script (for example) make the assertion reachable:
      ```
      local a = {}
      for i=1,7999 do
          a[i] = 1
      end
      return redis.call("lpush", "l", unpack(a))
      ```
      
      This is a regression because such a script would have worked before and now
      its crashing Redis. The solution is to clear the function arguments from the Lua
      stack which makes the original assumption true and the assertion unreachable.
      
      (cherry picked from commit 6b0b04f1)
      93c1d31d
    • Oran Agra's avatar
      fix invalid read on corrupt ziplist (#9831) · e38d0b5a
      Oran Agra authored
      If the last bytes in ziplist are corrupt and we decode from tail to head,
      we may reach slightly outside the ziplist.
      
      (cherry picked from commit a3a01429)
      e38d0b5a
    • Wen Hui's avatar
      Sentinel tls memory leak (#9753) · 94013b8e
      Wen Hui authored
      There was a memory leak when tls is used in Sentinels.
      The memory leak is noticed when some of the replicas are offline.
      
      (cherry picked from commit 2ce29e03)
      94013b8e
    • Itamar Haber's avatar
      Fixes LPOP/RPOP wrong replies when count is 0 (#9692) · 968cd2b9
      Itamar Haber authored
      Introduced in #8179, this fixes the command's replies in the 0 count edge case.
      [BREAKING] changes the reply type when count is 0 to an empty array (instead of nil)
      Moves LPOP ... 0 fast exit path after type check to reply with WRONGTYPE
      
      (cherry picked from commit 06dd202a)
      968cd2b9
    • Rafi Einstein's avatar
      Fix memory leak when there's a read error of module aux data from rdb. (#9705) · 19c2b179
      Rafi Einstein authored
      
      (cherry picked from commit 734cde7e)
      19c2b179
    • Oran Agra's avatar
      fix new cluster tests issues (#9657) · 3ba7c6ac
      Oran Agra authored
      Following #9483 the daily CI exposed a few problems.
      
      * The cluster creation code (uses redis-cli) is complicated to test with TLS enabled.
        for now i'm just skipping them since the tests we run there don't really need that kind of coverage
      * cluster port binding failures
        note that `find_available_port` already looks for a free cluster port
        but the code in `wait_server_started` couldn't detect the failure of binding
        (the text it greps for wasn't found in the log)
      
      (cherry picked from commit 7d6744c7)
      3ba7c6ac
    • qetu3790's avatar
      Release clients blocked on module commands in cluster resharding and down state (#9483) · 936ee017
      qetu3790 authored
      
      
      Prevent clients from being blocked forever in cluster when they block with their own module command
      and the hash slot is migrated to another master at the same time.
      These will get a redirection message when unblocked.
      Also, release clients blocked on module commands when cluster is down (same as other blocked clients)
      
      This commit adds basic tests for the main (non-cluster) redis test infra that test the cluster.
      This was done because the cluster test infra can't handle some common test features,
      but most importantly we only build the test modules with the non-cluster test suite.
      
      note that rather than really supporting cluster operations by the test infra, it was added (as dup code)
      in two files, one for module tests and one for non-modules tests, maybe in the future we'll refactor that.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      (cherry picked from commit 4962c552)
      936ee017
    • DarrenJiang13's avatar
      add missed error counting (#9646) · 8a0515d9
      DarrenJiang13 authored
      * add: add missed error counting in sentinel.c and cluster.c
      
      (cherry picked from commit aa6deff0)
      8a0515d9
    • Huang Zhw's avatar
      Make tracking invalidation messages always after command's reply (#9422) · 0fb96d55
      Huang Zhw authored
      Tracking invalidation messages were sometimes sent in inconsistent order,
      before the command's reply rather than after.
      In addition to that, they were sometimes embedded inside other commands
      responses, like MULTI-EXEC and MGET.
      
      (cherry picked from commit fd135f3e)
      0fb96d55
    • meir's avatar
      Protect any table which is reachable from globals and added globals allow list. · 11b602fb
      meir authored
      The allow list is done by setting a metatable on the global table before initializing
      any library. The metatable set the `__newindex` field to a function that check
      the allow list before adding the field to the table. Fields which is not on the
      allow list are simply ignored.
      
      After initialization phase is done we protect the global table and each table
      that might be reachable from the global table. For each table we also protect
      the table metatable if exists.
      11b602fb
    • meir's avatar
      Protect globals of evals scripts. · b2ce3719
      meir authored
      Use the new `lua_enablereadonlytable` Lua API to protect the global tables of
      evals scripts. The implemetation is easy, we simply call `lua_enablereadonlytable`
      on the global table to turn it into a readonly table.
      b2ce3719
    • meir's avatar
      Move user eval function to be located on Lua registry. · 21414ad4
      meir authored
      Today, Redis wrap the user Lua code with a Lua function.
      For example, assuming the user code is:
      
      ```
      return redis.call('ping')
      ```
      
      The actual code that would have sent to the Lua interpreter was:
      
      ```
      f_b3a02c833904802db9c34a3cf1292eee3246df3c() return redis.call('ping') end
      ```
      
      The wraped code would have been saved on the global dictionary with the
      following name: `f_<script sha>` (in our example `f_b3a02c833904802db9c34a3cf1292eee3246df3c`).
      
      This approach allows one user to easily override the implementation a another user code, example:
      
      ```
      f_b3a02c833904802db9c34a3cf1292eee3246df3c = function() return 'hacked' end
      ```
      
      Running the above code will cause `evalsha b3a02c833904802db9c34a3cf1292eee3246df3c 0` to return
      hacked although it should have returned `pong`.
      
      Another disadventage is that Redis basically runs code on the loading (compiling) phase without been
      aware of it. User can do code injection like this:
      
      ```
      return 1 end <run code on compling phase> function() return 1
      ```
      
      The wraped code will look like this and the entire `<run code on compling phase>` block will run outside
      of eval or evalsha context:
      
      ```
      f_<sha>() return 1 end <run code on compling phase> function() return 1 end
      ```
      21414ad4
  2. 11 Apr, 2022 1 commit
  3. 04 Oct, 2021 25 commits
    • Oran Agra's avatar
      Redis 6.2.6 · 4930d19e
      Oran Agra authored
      4930d19e
    • Yunier Pérez's avatar
      Allow to override OPENSSL_PREFIX (#9567) · 664c7b36
      Yunier Pérez authored
      While the original issue was on Linux, this should work for other
      platforms as well.
      664c7b36
    • Yossi Gottlieb's avatar
      c390972c
    • Oran Agra's avatar
      Fix stream sanitization for non-int first value (#9553) · 73d286d5
      Oran Agra authored
      This was recently broken in #9321 when we validated stream IDs to be
      integers but did that after to the stepping next record instead of before.
      
      (cherry picked from commit 5a4ab7c7)
      73d286d5
    • David CARLIER's avatar
      TLS build fix on OpenBSD when built with LibreSSL. (#9486) · 4cbccab2
      David CARLIER authored
      
      (cherry picked from commit 418c2e79)
      4cbccab2
    • yvette903's avatar
      Fix: client pause uses an old timeout (#9477) · 23ed37dd
      yvette903 authored
      A write request may be paused unexpectedly because `server.client_pause_end_time` is old.
      
      **Recreate this:**
      redis-cli -p 6379
      127.0.0.1:6379> client pause 500000000 write
      OK
      127.0.0.1:6379> client unpause
      OK
      127.0.0.1:6379> client pause 10000 write
      OK
      127.0.0.1:6379> set key value
      
      The write request `set key value` is paused util  the timeout of 500000000 milliseconds was reached.
      
      **Fix:**
      reset `server.client_pause_end_time` = 0 in `unpauseClients`
      
      (cherry picked from commit f560531d)
      23ed37dd
    • zhaozhao.zz's avatar
      Fix wrong offset when replica pause (#9448) · 9b25484a
      zhaozhao.zz authored
      When a replica paused, it would not apply any commands event the command comes from master, if we feed the non-applied command to replication stream, the replication offset would be wrong, and data would be lost after failover(since replica's `master_repl_offset` grows but command is not applied).
      
      To fix it, here are the changes:
      * Don't update replica's replication offset or propagate commands to sub-replicas when it's paused in `commandProcessed`.
      * Show `slave_read_repl_offset` in info reply.
      * Add an assert to make sure master client should never be blocked unless pause or module (some modules may use block way to do background (parallel) processing and forward original block module command to the replica, it's not a good way but it can work, so the assert excludes module now, but someday in future all modules should rewrite block command to propagate like what `BLPOP` does).
      
      (cherry picked from commit 1b83353d)
      9b25484a
    • guybe7's avatar
      Fix two minor bugs (MIGRATE key args and getKeysUsingCommandTable) (#9455) · c936f801
      guybe7 authored
      1. MIGRATE has a potnetial key arg in argv[3]. It should be reflected in the command table.
      2. getKeysUsingCommandTable should never free getKeysResult, it is always freed by the caller)
         The reason we never encountered this double-free bug is that almost always getKeysResult
         uses the statis buffer and doesn't allocate a new one.
      
      (cherry picked from commit 6aa2285e)
      c936f801
    • sundb's avatar
      Fix the timing of read and write events under kqueue (#9416) · 694a869e
      sundb authored
      Normally we execute the read event first and then the write event.
      When the barrier is set, we will do it reverse.
      However, under `kqueue`, if an `fd` has both read and write events,
      reading the event using `kevent` will generate two events, which will
      result in uncontrolled read and write timing.
      
      This also means that the guarantees of AOF `appendfsync` = `always` are
      not met on MacOS without this fix.
      
      The main change to this pr is to cache the events already obtained when reading
      them, so that if the same `fd` occurs again, only the mask in the cache is updated,
      rather than a new event is generated.
      
      This was exposed by the following test failure on MacOS:
      ```
      *** [err]: AOF fsync always barrier issue in tests/integration/aof.tcl
      Expected 544 != 544 (context: type eval line 26 cmd {assert {$size1 != $size2}} proc ::test)
      ```
      
      (cherry picked from commit 306a5ccd)
      694a869e
    • sundb's avatar
      a2e8a3a2
    • Wang Yuan's avatar
      Fix the wrong detection of sync_file_range system call (#9371) · 2f8ec0e0
      Wang Yuan authored
      If we want to check `defined(SYNC_FILE_RANGE_WAIT_BEFORE)`, we should include fcntl.h.
      otherwise, SYNC_FILE_RANGE_WAIT_BEFORE is not defined, and there is alway not `sync_file_range` system call.
      Introduced by #8532
      
      (cherry picked from commit 8edc3cd6)
      2f8ec0e0
    • sundb's avatar
      Sanitize dump payload: handle remaining empty key when RDB loading and restore command (#9349) · 09c63c45
      sundb authored
      This commit mainly fixes empty keys due to RDB loading and restore command,
      which was omitted in #9297.
      
      1) When loading quicklsit, if all the ziplists in the quicklist are empty, NULL will be returned.
          If only some of the ziplists are empty, then we will skip the empty ziplists silently.
      2) When loading hash zipmap, if zipmap is empty, sanitization check will fail.
      3) When loading hash ziplist, if ziplist is empty, NULL will be returned.
      4) Add RDB loading test with sanitize.
      
      (cherry picked from commit cbda4929)
      09c63c45
    • DarrenJiang13's avatar
      [BUGFIX] Add some missed error statistics (#9328) · 1ed0f049
      DarrenJiang13 authored
      add error counting for some missed behaviors.
      
      (cherry picked from commit 43eb0ce3)
      1ed0f049
    • Oran Agra's avatar
      Improvements to corrupt payload sanitization (#9321) · 4b04ca0b
      Oran Agra authored
      
      
      Recently we found two issues in the fuzzer tester: #9302 #9285
      After fixing them, more problems surfaced and this PR (as well as #9297) aims to fix them.
      
      Here's a list of the fixes
      - Prevent an overflow when allocating a dict hashtable
      - Prevent OOM when attempting to allocate a huge string
      - Prevent a few invalid accesses in listpack
      - Improve sanitization of listpack first entry
      - Validate integrity of stream consumer groups PEL
      - Validate integrity of stream listpack entry IDs
      - Validate ziplist tail followed by extra data which start with 0xff
      Co-authored-by: default avatarsundb <sundbcn@gmail.com>
      (cherry picked from commit 0c90370e)
      4b04ca0b
    • sundb's avatar
      Sanitize dump payload: fix empty keys when RDB loading and restore command (#9297) · 2f541072
      sundb authored
      
      
      When we load rdb or restore command, if we encounter a length of 0, it will result in the creation of an empty key.
      This could either be a corrupt payload, or a result of a bug (see #8453 )
      
      This PR mainly fixes the following:
      1) When restore command will return `Bad data format` error.
      2) When loading RDB, we will silently discard the key.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      (cherry picked from commit 8ea777a6)
      2f541072
    • menwen's avatar
      Add latency monitor sample when key is deleted via lazy expire (#9317) · e34f06ae
      menwen authored
      Fix that there is no sample latency after the key expires via expireIfNeeded().
      Some refactoring for shared code.
      
      (cherry picked from commit ca559819)
      e34f06ae
    • Huang Zhw's avatar
      When redis-cli received ASK, it didn't handle it (#8930) · 8892b5cf
      Huang Zhw authored
      
      
      When redis-cli received ASK, it used string matching wrong and didn't
      handle it.
      
      When we access a slot which is in migrating state, it maybe
      return ASK. After redirect to the new node, we need send ASKING
      command before retry the command.  In this PR after redis-cli receives
      ASK, we send a ASKING command before send the origin command
      after reconnecting.
      
      Other changes:
      * Make redis-cli -u and -c (unix socket and cluster mode) incompatible
        with one another.
      * When send command fails, we avoid the 2nd reconnect retry and just
        print the error info. Users will decide how to do next.
        See #9277.
      * Add a test faking two redis nodes in TCL to just send ASK and OK in
        redis protocol to test ASK behavior.
      Co-authored-by: default avatarViktor Söderqvist <viktor.soderqvist@est.tech>
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      (cherry picked from commit cf61ad14)
      8892b5cf
    • Binbin's avatar
      redis-cli: Sleep for a while in each cliConnect when we got connect error in cluster mode. (#8884) · 9da232c0
      Binbin authored
      
      
      There's an infinite loop when redis-cli fails to connect in cluster mode.
      This commit adds a 1 second sleep to prevent flooding the console with errors.
      It also adds a specific error print in a few places that could have error without printing anything.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      (cherry picked from commit 8351a10b)
      9da232c0
    • Huang Zhw's avatar
      redis-cli when SELECT fails, we should reset dbnum to 0 (#8898) · 1ce06604
      Huang Zhw authored
      
      
      when SELECT fails, we should reset dbnum to 0, so the prompt will not
      display incorrectly.
      
      Additionally when SELECT and HELLO fail, we output message to inform
      it.
      
      Add config.input_dbnum which means the dbnum about to select.
      And config.dbnum means currently selected dbnum. When users succeed to
      select db, config.dbnum and config.input_dbnum will be the same. When
      users select db failed, config.input_dbnum will be kept. Next time if users
      auth success, config.input_dbnum will be automatically selected.
      When reconnect, we should select the origin dbnum.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      (cherry picked from commit 6b475989)
      1ce06604
    • Binbin's avatar
      GEO* STORE with empty src key delete the dest key and return 0, not empty array (#9271) · 530c70b0
      Binbin authored
      
      
      With an empty src key, we need to deal with two situations:
      1. non-STORE: We should return emptyarray.
      2. STORE: Try to delete the store key and return 0.
      
      This applies to both GEOSEARCHSTORE (new to v6.2), and
      also GEORADIUS STORE (which was broken since forever)
      
      This pr try to fix #9261. i.e. both STORE variants would have behaved
      like the non-STORE variants when the source key was missing,
      returning an empty array and not deleting the destination key,
      instead of returning 0, and deleting the destination key.
      
      Also add more tests for some commands.
      - GEORADIUS: wrong type src key, non existing src key, empty search,
        store with non existing src key, store with empty search
      - GEORADIUSBYMEMBER: wrong type src key, non existing src key,
        non existing member, store with non existing src key
      - GEOSEARCH: wrong type src key, non existing src key, empty search,
        frommember with non existing member
      - GEOSEARCHSTORE: wrong type key, non existing src key,
        fromlonlat with empty search, frommember with non existing member
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      (cherry picked from commit 86555ae0)
      530c70b0
    • YiyuanGUO's avatar
      dadc67a9
    • Oran Agra's avatar
      Fix ziplist and listpack overflows and truncations (CVE-2021-32627, CVE-2021-32628) · 2775a352
      Oran Agra authored
      - fix possible heap corruption in ziplist and listpack resulting by trying to
        allocate more than the maximum size of 4GB.
      - prevent ziplist (hash and zset) from reaching size of above 1GB, will be
        converted to HT encoding, that's not a useful size.
      - prevent listpack (stream) from reaching size of above 1GB.
      - XADD will start a new listpack if the new record may cause the previous
        listpack to grow over 1GB.
      - XADD will respond with an error if a single stream record is over 1GB
      - List type (ziplist in quicklist) was truncating strings that were over 4GB,
        now it'll respond with an error.
      2775a352
    • meir@redislabs.com's avatar
      Fix invalid memory write on lua stack overflow {CVE-2021-32626} · 8f241ab3
      meir@redislabs.com authored
      When LUA call our C code, by default, the LUA stack has room for 20
      elements. In most cases, this is more than enough but sometimes it's not
      and the caller must verify the LUA stack size before he pushes elements.
      
      On 3 places in the code, there was no verification of the LUA stack size.
      On specific inputs this missing verification could have lead to invalid
      memory write:
      1. On 'luaReplyToRedisReply', one might return a nested reply that will
         explode the LUA stack.
      2. On 'redisProtocolToLuaType', the Redis reply might be deep enough
         to explode the LUA stack (notice that currently there is no such
         command in Redis that returns such a nested reply, but modules might
         do it)
      3. On 'ldbRedis', one might give a command with enough arguments to
         explode the LUA stack (all the arguments will be pushed to the LUA
         stack)
      
      This commit is solving all those 3 issues by calling 'lua_checkstack' and
      verify that there is enough room in the LUA stack to push elements. In
      case 'lua_checkstack' returns an error (there is not enough room in the
      LUA stack and it's not possible to increase the stack), we will do the
      following:
      1. On 'luaReplyToRedisReply', we will return an error to the user.
      2. On 'redisProtocolToLuaType' we will exit with panic (we assume this
         scenario is rare because it can only happen with a module).
      3. On 'ldbRedis', we return an error.
      8f241ab3
    • meir@redislabs.com's avatar
      Fix protocol parsing on 'ldbReplParseCommand' (CVE-2021-32672) · 3e09be56
      meir@redislabs.com authored
      The protocol parsing on 'ldbReplParseCommand' (LUA debugging)
      Assumed protocol correctness. This means that if the following
      is given:
      *1
      $100
      test
      The parser will try to read additional 94 unallocated bytes after
      the client buffer.
      This commit fixes this issue by validating that there are actually enough
      bytes to read. It also limits the amount of data that can be sent by
      the debugger client to 1M so the client will not be able to explode
      the memory.
      3e09be56
    • Oran Agra's avatar
      Prevent unauthenticated client from easily consuming lots of memory (CVE-2021-32675) · 757f8f77
      Oran Agra authored
      This change sets a low limit for multibulk and bulk length in the
      protocol for unauthenticated connections, so that they can't easily
      cause redis to allocate massive amounts of memory by sending just a few
      characters on the network.
      The new limits are 10 arguments of 16kb each (instead of 1m of 512mb)
      757f8f77