1. 27 Apr, 2022 16 commits
    • 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
    • Binbin's avatar
      Add tests to cover EXPIRE overflow fix (#9839) · 8fca090e
      Binbin authored
      In #8287, some overflow checks have been added. But when
      `when *= 1000` overflows, it will become a positive number.
      And the check not able to catch it. The key will be added with
      a short expiration time and will deleted a few seconds later.
      
      In #9601, will check the overflow after `*=` and return an
      error first, and avoiding this situation.
      
      In this commit, added some tests to cover those code paths.
      Found it in #9825, and close it.
      
      (cherry picked from commit 9273d09d)
      8fca090e
    • 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
    • Yossi Gottlieb's avatar
      hiredis: improve calloc() overflow fix. (#9630) · 21ab5d4f
      Yossi Gottlieb authored
      Cherry pick a more complete fix to 0215324a that also doesn't leak
      memory from latest hiredis.
      
      (cherry picked from commit 922ef86a)
      21ab5d4f
    • 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
    • Binbin's avatar
      Add missing pause tcl test to test_helper.tcl (#9158) · f5d8a369
      Binbin authored
      * Add keyname tags to avoid CROSSSLOT errors in external server CI
      * Use new wait_for_blocked_clients_count in pause.tcl
      
      (cherry picked from commit 5dddf496)
      f5d8a369
    • 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
    • meir's avatar
      Added support for Lua readonly tables. · 13c1e1f2
      meir authored
      The new feature can be turned off and on using the new `lua_enablereadonlytable` Lua API.
      
      (cherry picked from commit 92b5098b87e2d0880a530899119524bf1dfbc332)
      13c1e1f2
  2. 11 Apr, 2022 1 commit
  3. 04 Oct, 2021 23 commits
    • Oran Agra's avatar
      Redis 6.2.6 · 4930d19e
      Oran Agra authored
      4930d19e
    • Oran Agra's avatar
      corrupt-dump-fuzzer test, avoid creating junk keys (#9302) · aba95175
      Oran Agra authored
      The execution of the RPOPLPUSH command by the fuzzer created junk keys,
      that were later being selected by RANDOMKEY and modified.
      This also meant that lists were statistically tested more than other
      files.
      
      Fix the fuzzer not to pass junk key names to RPOPLPUSH, and add a check
      that detects that new keys are not added by the fuzzer to detect future
      similar issues.
      
      (cherry picked from commit 3f3f678a)
      aba95175
    • sundb's avatar
      Fix missing check for sanitize_dump in corrupt-dump-fuzzer test (#9285) · 7540708a
      sundb authored
      this means the assertion that checks that when deep sanitization is enabled,
      there are no crashes, was missing.
      
      (cherry picked from commit 3db0f1a2)
      7540708a
    • 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
    • Madelyn Olson's avatar
      Add test verifying PUBSUB NUMPAT behavior (#9209) · 49f8f438
      Madelyn Olson authored
      
      (cherry picked from commit 8b8f05c8)
      49f8f438
    • 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
    • Viktor Söderqvist's avatar
      redis-cli ASK redirect test: Add retry loop to fix timing issue (#9315) · 77386ae0
      Viktor Söderqvist authored
      
      (cherry picked from commit 1c59567a)
      77386ae0
    • Oran Agra's avatar
      Skip new redis-cli ASK test in TLS mode (#9312) · 0c959294
      Oran Agra authored
      
      (cherry picked from commit 52df350f)
      0c959294
    • 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