Security: fix low-severity findings (v0.11.8) - #40
Merged
Conversation
Address the Low findings from the security audit.
- ACL WHOAMI: return the connection's actual authenticated user instead
of a hardcoded "default" (answered in the connection layer).
- Channel ACLs: enforce a user's channel patterns on SUBSCRIBE/PSUBSCRIBE/
PUBLISH (and shard variants), previously unchecked.
- Config: custom Debug impl redacts requirepass so it can't leak via a
{:?} log line.
- Compression: bound decompress() output size (512MB) to reject a
decompression bomb before allocating.
- AOF replay: duration_since(UNIX_EPOCH) uses unwrap_or_default instead of
unwrap (no panic on a pre-1970 clock).
- Scripting caches: recover poisoned mutexes instead of panicking on
.lock().unwrap().
Adds tests for channel extraction and the decompression-size guard.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the Low findings from the security audit. Version bumped to 0.11.8.
cargo checkis clean (no warnings); 87 unit tests + all integration tests pass except the two pubsub integration tests in the untracked WIPtests/dir that fail identically onmain(pre-existing, unrelated).Findings addressed
24.
ACL WHOAMIhardcoded "default". Now answered in the connection layer and returns the actual authenticated username.25. Channel ACLs never enforced. A user's
channel_patternsare now checked onSUBSCRIBE/PSUBSCRIBE/PUBLISH(and the shard variants) viacommand_channels+is_channel_allowed, mirroring the key-ACL enforcement. The default user (&*) is unaffected.26.
ConfigDebugleaked plaintextrequirepass. Replaced the derivedDebugwith a manual impl that redactsrequirepass(shown asSome("***")), so it can't land in a{:?}log line.28. Decompression-bomb shape in
compression.rs.decompress()now validates both the header's declared length and the LZ4 stream's embedded size prefix against a 512 MB cap before decompressing. (Still no non-test callers, so this is defensive.)29. Small robustness items.
duration_since(UNIX_EPOCH).unwrap()→unwrap_or_default()(no panic on a pre-1970 clock).ScriptCache/FunctionLibrary.lock().unwrap()→ poison-recoveringunwrap_or_else(|e| e.into_inner()).Not changed (documented residuals)
ACL SETUSER user #<sha256hex>andACL GETUSER(which returns the hash) both assume unsalted SHA-256. Salting would break compatibility, which is a core project goal.rustls-pemfileunmaintained (RUSTSEC-2025-0134) — deferred. This is a maintenance advisory, not a vulnerability. Therustls-pki-types 1.14.0already in the tree exposes no PEM API, so replacing it would require a broader upgrade of security-sensitive TLS parsing; not worth the risk for a Low.SAVE/BGSAVEhardcodedump.rdb/appendonly.aof. Using the configured filename requires threadingConfigthroughCommandContext(touched by every handler); deferred as disproportionate for a Low that only matters when the operator renamed the file. The values still match the config defaults.PSUBSCRIBEpatterns are checked against the user's channel patterns with glob matching (approximate), not Redis's exact literal-pattern rule.Adds
command_channelsand decompression-guard unit tests.