1. 27 Apr, 2022 7 commits
    • 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 26 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
    • Oran Agra's avatar
      Fix Integer overflow issue with intsets (CVE-2021-32687) · b1149a49
      Oran Agra authored
      The vulnerability involves changing the default set-max-intset-entries
      configuration parameter to a very large value and constructing specially
      crafted commands to manipulate sets
      b1149a49
  4. 21 Jul, 2021 6 commits
    • Oran Agra's avatar
      Redis 6.2.5 · db09f6eb
      Oran Agra authored
      db09f6eb
    • Huang Zhw's avatar
      On 32 bit platform, the bit position of GETBIT/SETBIT/BITFIELD/BITCOUNT,BITPOS... · 835d15b5
      Huang Zhw authored
      On 32 bit platform, the bit position of GETBIT/SETBIT/BITFIELD/BITCOUNT,BITPOS may overflow (see CVE-2021-32761) (#9191)
      
      GETBIT, SETBIT may access wrong address because of wrap.
      BITCOUNT and BITPOS may return wrapped results.
      BITFIELD may access the wrong address but also allocate insufficient memory and segfault (see CVE-2021-32761).
      
      This commit uses `uint64_t` or `long long` instead of `size_t`.
      related https://github.com/redis/redis/pull/8096
      
      At 32bit platform:
      > setbit bit 4294967295 1
      (integer) 0
      > config set proto-max-bulk-len 536870913
      OK
      > append bit "\xFF"
      (integer) 536870913
      > getbit bit 4294967296
      (integer) 0
      
      When the bit index is larger than 4294967295, size_t can't hold bit index. In the past,  `proto-max-bulk-len` is limit to 536870912, so there is no problem.
      
      After this commit, bit position is stored in `uint64_t` or `long long`. So when `proto-max-bulk-len > 536870912`, 32bit platforms can still be correct.
      
      For 64bit platform, this problem still exists. The major reason is bit pos 8 times of byte pos. When proto-max-bulk-len is very larger, bit pos may overflow.
      But at 64bit platform, we don't have so long string. So this bug may never happen.
      
      Additionally this commit add a test cost `512MB` memory which is tag as `large-memory`. Make freebsd ci and valgrind ci ignore this test.
      
      (cherry picked from commit 71d45287)
      835d15b5
    • Huang Zhw's avatar
      Remove testmodule in src/modules/Makefile. (#9250) · 8d613495
      Huang Zhw authored
      src/modules make failed. As in #3718 testmodule.c was removed. But the makefile was not updated
      
      (cherry picked from commit d54c9086)
      8d613495
    • Oran Agra's avatar
      Adjustments to recent RM_StringTruncate fix (#3718) (#9125) · 37b0f361
      Oran Agra authored
      - Introduce a new sdssubstr api as a building block for sdsrange.
        The API of sdsrange is many times hard to work with and also has
        corner case that cause bugs. sdsrange is easy to work with and also
        simplifies the implementation of sdsrange.
      - Revert the fix to RM_StringTruncate and just use sdssubstr instead of
        sdsrange.
      - Solve valgrind warnings from the new tests introduced by the previous
        PR.
      
      (cherry picked from commit ae418eca)
      37b0f361
    • Huang Zhw's avatar
      Fix missing separator in module info line (usedby and using lists) (#9241) · 68661171
      Huang Zhw authored
      
      
      Fix module info genModulesInfoStringRenderModulesList lack separator when there's more than one module in the list.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      (cherry picked from commit 1895e134)
      68661171
    • Binbin's avatar
      SMOVE only notify dstset when the addition is successful. (#9244) · b6225371
      Binbin authored
      in case dest key already contains the member, the dest key isn't modified, so the command shouldn't invalidate watch.
      
      (cherry picked from commit 11dc4e59)
      b6225371