I reviewed valkey 9.0.3-0ubuntu1 as checked into resolute. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.

valkey is the distributed key-value store which can be used as a database,
cache, message broker and for multiple other purposes.

- CVE History
  - 12 CVEs in the cve tracker and no any outstanding CVEs. Dependencies are 
bundled 
    with the package itself which is not ideal. fast_float, 
fast_float_c_interface,
    fpconv, hdr_histogram, jemalloc, libvalkey, linenoise and lua are bundled 
with the package.
    
  - We had quite a few CVEs in the past for lua scripting engine and they were 
released as part 
    of valkey itself which at least makes it a bit easier for tracking and 
patching. 
    
  - Going through the release notes for valkey, I found fixes for memory 
leak(#2362,#2288,#2973),
    memory corruption(#2137) and use-after-free(#2257) but they do not have a 
corresponding CVE entries.

- Build-Depends
  - From `debian/control` file:
    - debhelper-compat
    - libhiredis-dev
    - libjemalloc-dev
    - liblua5.1-dev
    - liblzf-dev
    - libssl-dev
    - libsystemd-dev
    - lua-bitop-dev
    - lua-cjson-dev
    - openssl
    - pkgconf
    - procps
    - tcl
    - tcl-tls
  - Packages only in main: libssl-dev,  openssl. pkgconf, tcl
  - `openssl` used for TLS, `libhiredis-dev` for redis client networking,
    `libjemalloc-dev` as memory allocator, `liblzf-dev` for compression and
    `lua-cjson-dev` used for json encoding and parsing

- pre/post inst/rm scripts
  - Present and looks okay.

- init scripts
  - Present for both `valkey-sentinel` and `valkey-server`.
    These act as fallback for systemd and not used by default

- systemd units
  - Systemd units present for `valkey-server` and `valkey-sentinel`.
  - Both run as `valkey` user/group.
  - By default,
      valkey-sentinel:
        - listens on all interface
        - Runs on port 26379
        - Where value for `daemonize` is set as yes on `sentinel.conf`, the 
ExecStart command on the service file is: `ExecStart=/usr/bin/valkey-sentinel 
/etc/valkey/sentinel.conf --supervised systemd --daemonize no`
      valkey-server:
        - Only listens on localhost
        - Runs on port 6379
        - Supervised by systemd and is not daemonized, ExecStart: 
`/usr/bin/valkey-server /etc/valkey/valkey.conf --supervised systemd 
--daemonize no`

- dbus services
  - None

- setuid binaries
  - None

- binaries in PATH
  - valkey-benchmark
  - valkey-check-aof
  - valkey-check-rdb
  - valkey-cli
  - valkey-sentinel -> valkey-check-rdb
  - valkey-server -> valkey-check-rdb

- sudo fragments
  - None
- polkit files
  - None
- udev rules
  - None

- unit tests / autopkgtests
  - During build time, around 4590 tests divided among unit and integrations 
are run successfully.
    Autopkgtests are present and ran successfully.
- cron jobs
  - None
- Build logs
  - Few warnings present, nothing concerning

- Processes spawned
  - execve calls at two different places
    - first one is to restart the valkey server with the same parameters with 
which it was started
    - Second one is to run sentinel scripts. Client are able to run scripts but 
this is disabled in
      default configuration. The other way to run this is by setting directives 
in the config files.
    - Both look okay.

- Memory management
  - A lot of instances of memcpy and memmove and a lot of pointer arithemtics.
  - Heavy use of allocator wrappers (zmalloc, zcalloc, zrealloc) instead of raw 
functions.
  - Looks okay.

- File IO
  - Paths are mostly derived from config files, hardcoded and created 
temporarily in some cases.
  - `0666` is used for creating temporary files and `0644` for other files.

- Logging
  - Wrapper used for logging which logs the log level and log messages.
  - A lot of warnings are errors are logged and overall usage looks ok.
  - Logs are also stored in `/var/log/valkey` for both sentinel and server.
    Log files are owned by `valkey:adm` with 660 permissions.

- Environment variable usage
  - Ok

- Use of privileged functions
  - Usage looks good.
  - Use of functions like chmod, chown, fchmod and ioctl
  - ioctl used for reading terminal size to display output and to toggle the 
blocking/nonblocking behaviour of the socket
  - Use of chmod and chown on the socket looks ok.
  - fchmod used to change the permission of a temporary config file(0644) and 
to set linenoise history file to 0600 

- Use of cryptography / random number sources etc
  - TLS present and optional, not enabled by default
  - Openssl is used for TLS, `RAND_poll()` from openssl used for generating 
random seed
  - TLSv1 and TLSv1.1 are still supported.
  - Overall implementation looks good.

- Use of temp files
  - Ok

- Use of networking
  - Heavy use of networking and socket.
  - Communication between client and server is done over TCP using REdis 
Serialization Protocol (RESP)
  - Nodes in valkey cluster are connected using Valkey Cluster Bus and use 
gossip protocol to communicate.
  - valkey being a key-value store, command arguments are treated as opaque 
binary-safe strings.

- Use of WebKit
  - None
- Use of PolicyKit
  - None

- Any significant cppcheck results
  - Most of the results regarding unknown macro and 'Uninitialized variable' 
are false positives.
  - One potential Null pointer deference in `usUntilEarliestTimer` function

- Any significant Coverity results
  - A few stick out but nothing too major:
    - `VM_ListInsert` with potential null pointer deference
    - `CLUSTER_MANAGER_PRINT_REPLY_ERROR(n, err)`: err is a `char **` but 
looking at the 
        function definition, char * is expected which might print garbage value.
    - `valkey-cli.c:5051`:  unsigned long(8 bytes in x64) used for length and 
using `%d`(4 bytes) to print the value
  - STACK_USE: Few stack allocations but no real issue, with highest being 1 
MB(only created once)
      and other in range of 128-512 KB, nothing too concerning
  - TOCTOU: Techically valid cases, but irrelevant
  - A lof ot `CHECKED_RETURN` entries and most of them that I have checked are 
false positives.
  - CONSTANT_EXPRESSION_RESULT: No issue
  - WEAK_CRYPTO: A lot of entries but nothing is used for sensitive purposes.
  - USE_AFTER_FREE: False positives only

- Any significant shellcheck results
  - Looks okay.
  - A lot of scripts are used for testing purposes and other related to 
configure script for jemalloc.

- Any significant bandit results
  - All issues are related to the tests.
- Any significant govulncheck results
  - N/A

- Any significant Semgrep results
  - Nothing significant
  - SHA1 only used for testing purposes.
  - system() call present which only allows to execute os_clock and is a false 
positive.

Security team ACKs for promoting valkey to main. It would be great if valkey 
could make use of
system packages rather than using vendored dependencies.

** Changed in: valkey (Ubuntu)
       Status: New => In Progress

** Changed in: valkey (Ubuntu)
     Assignee: Ubuntu Security Team (ubuntu-security) => (unassigned)

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/2142907

Title:
  [MIR] valkey

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/valkey/+bug/2142907/+subscriptions


-- 
ubuntu-bugs mailing list
[email protected]
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to