Skip to content

Commit 442cdf0

Browse files
SnidercodexCerberus
committed
fix(cache): close threat-model audit findings — pattern length + symlink escape (Cerberus)
Two reachable findings closed via Cerberus-supervised codex audit (Mantis #923): 1. Untrusted-key DoS via Invalidate (medium): Invalidate accepted callback-returned glob patterns without bounds, which then drove keysByPattern through a full key listing. New ensureSafePattern enforces a 4096-byte cap before listing. Repro: TestCache_Invalidate_UntrustedPatternLength_Bad 2. Path traversal via symlink escape (high): ensureSafeKey rejects byte-pattern traversal (.., null byte, leading /, control chars) but did not catch runtime-state symlink-following — a symlinked subdirectory inside baseDir could redirect an otherwise safe key outside the cache root. New ensureNoSymlinkPath walks each path segment via os.Lstat and rejects symlink components. Repro: TestCache_Path_PathTraversalSymlink_Bad Section 3 (TOCTOU on Cache.mu / Invalidate-while-OnInvalidate) is documented in threats.md as TBD — codex budget exhausted before reaching it. Filed as a separate follow-up ticket per scope-expansion discipline. Verification: go vet clean; go test -race -count=1 passes (1.341s). Workspace mode required (pre-existing task #28 forge dep break affects GOWORK=off; not introduced by this commit). Found-by: Cerberus (mechanism-tier audit) Audit-by: codex (supervised by Cerberus) Closes tasks.lthn.sh/view.php?id=923 Co-Authored-By: Codex <noreply@openai.com> Co-Authored-By: Cerberus <cerberus@lthn.ai> Co-Authored-By: Virgil <virgil@lethean.io>
1 parent 140b863 commit 442cdf0

3 files changed

Lines changed: 151 additions & 0 deletions

File tree

cache.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ const DefaultTTL = 1 * time.Hour
2929

3030
const (
3131
maxCacheKeyBytes = 4096
32+
maxCachePatternBytes = 4096
3233
maxCacheNameBytes = 255
3334
maxCachedRequestURLBytes = 8192
3435
maxCachedRequestMethodBytes = 32
@@ -144,6 +145,9 @@ func (cache *Cache) Path(key string) (string, error) {
144145
if path != baseDir && !core.HasPrefix(path, pathPrefix) {
145146
return "", core.E("cache.Path", "invalid cache key: path traversal attempt", nil)
146147
}
148+
if err := ensureNoSymlinkPath(baseDir, path); err != nil {
149+
return "", core.E("cache.Path", "invalid cache key: symlink escape attempt", err)
150+
}
147151

148152
return path, nil
149153
}
@@ -520,6 +524,10 @@ func (cache *Cache) collectJSONKeys(prefix string) ([]string, error) {
520524
}
521525

522526
func (cache *Cache) keysByPattern(pattern string) ([]string, error) {
527+
if err := ensureSafePattern(pattern); err != nil {
528+
return nil, err
529+
}
530+
523531
allKeys, err := cache.listJSONKeys()
524532
if err != nil {
525533
return nil, err
@@ -763,6 +771,60 @@ func ensureSafeKey(key string) error {
763771
return nil
764772
}
765773

774+
func ensureSafePattern(pattern string) error {
775+
if pattern == "" {
776+
return core.E("cache.validatePattern", "invalid empty pattern", nil)
777+
}
778+
if len(pattern) > maxCachePatternBytes {
779+
return core.E("cache.validatePattern", "invalid pattern: too long", nil)
780+
}
781+
if core.Contains(pattern, "\\") || hasPathDangerousBytes(pattern) {
782+
return core.E("cache.validatePattern", "invalid pattern: contains control bytes", nil)
783+
}
784+
return nil
785+
}
786+
787+
func ensureNoSymlinkPath(baseDir, path string) error {
788+
if err := rejectSymlink(baseDir); err != nil {
789+
return err
790+
}
791+
792+
if path == baseDir {
793+
return nil
794+
}
795+
796+
rel := core.TrimPrefix(path, normalizePath(core.Concat(baseDir, pathSeparator())))
797+
if rel == path {
798+
return nil
799+
}
800+
801+
current := baseDir
802+
for _, part := range core.Split(rel, pathSeparator()) {
803+
if part == "" {
804+
continue
805+
}
806+
current = core.JoinPath(current, part)
807+
if err := rejectSymlink(current); err != nil {
808+
return err
809+
}
810+
}
811+
return nil
812+
}
813+
814+
func rejectSymlink(path string) error {
815+
info, err := os.Lstat(path)
816+
if err != nil {
817+
if os.IsNotExist(err) {
818+
return nil
819+
}
820+
return err
821+
}
822+
if info.Mode()&os.ModeSymlink != 0 {
823+
return core.E("cache.validatePath", "path contains symlink", nil)
824+
}
825+
return nil
826+
}
827+
766828
func hasPathDangerousBytes(s string) bool {
767829
for i := 0; i < len(s); i++ {
768830
if s[i] < 0x20 || s[i] == 0x7f {

cache_test.go

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,40 @@ func TestCache_Path_Bad(t *testing.T) {
273273
}
274274
}
275275

276+
func TestCache_Path_PathTraversalSymlink_Bad(t *testing.T) {
277+
tmpDir := t.TempDir()
278+
baseDir := core.JoinPath(tmpDir, "cache")
279+
outsideDir := core.JoinPath(tmpDir, "outside")
280+
linkPath := core.JoinPath(baseDir, "link")
281+
282+
if err := os.MkdirAll(baseDir, 0o755); err != nil {
283+
t.Fatalf("MkdirAll base failed: %v", err)
284+
}
285+
if err := os.MkdirAll(outsideDir, 0o755); err != nil {
286+
t.Fatalf("MkdirAll outside failed: %v", err)
287+
}
288+
if err := os.Symlink(outsideDir, linkPath); err != nil {
289+
t.Skipf("symlink not supported: %v", err)
290+
}
291+
292+
c, err := cache.New(coreio.Local, baseDir, time.Minute)
293+
if err != nil {
294+
t.Fatalf("New failed: %v", err)
295+
}
296+
297+
if _, err := c.Path("link/escaped"); err == nil {
298+
t.Fatal("expected Path to reject symlink traversal under baseDir")
299+
}
300+
if err := c.Set("link/escaped", "owned"); err == nil {
301+
t.Fatal("expected Set to reject symlink traversal under baseDir")
302+
}
303+
if _, err := os.Stat(core.JoinPath(outsideDir, "escaped.json")); err == nil {
304+
t.Fatal("expected escaped file not to be written outside baseDir")
305+
} else if !os.IsNotExist(err) {
306+
t.Fatalf("Stat outside file failed: %v", err)
307+
}
308+
}
309+
276310
func TestCache_Get_Good(t *testing.T) {
277311
c, _ := newTestCache(t, "/tmp/cache", time.Minute)
278312

@@ -1119,6 +1153,35 @@ func TestCache_Invalidate_Good(t *testing.T) {
11191153
}
11201154
}
11211155

1156+
func TestCache_Invalidate_UntrustedPatternLength_Bad(t *testing.T) {
1157+
c, _ := newTestCache(t, "/tmp/cache-invalidate-pattern-length", time.Minute)
1158+
1159+
if err := c.Set("dns/example.com/A", "record"); err != nil {
1160+
t.Fatalf("Set failed: %v", err)
1161+
}
1162+
1163+
c.OnInvalidate("dns.changed", func(trigger string) []string {
1164+
return []string{strings.Repeat("a", 4097)}
1165+
})
1166+
1167+
deleted, err := c.Invalidate("dns.changed")
1168+
if err == nil {
1169+
t.Fatal("expected Invalidate to reject an oversized pattern")
1170+
}
1171+
if deleted != 0 {
1172+
t.Fatalf("expected no deletions after rejecting oversized pattern, got %d", deleted)
1173+
}
1174+
1175+
var record string
1176+
found, err := c.Get("dns/example.com/A", &record)
1177+
if err != nil {
1178+
t.Fatalf("Get failed: %v", err)
1179+
}
1180+
if !found || record != "record" {
1181+
t.Fatalf("expected entry to remain, found=%v record=%q", found, record)
1182+
}
1183+
}
1184+
11221185
func TestCache_Invalidate_PrefixWildcardDoesNotMatchBarePrefix(t *testing.T) {
11231186
c, _ := newTestCache(t, "/tmp/cache-invalidate-prefix", time.Minute)
11241187

threats.md

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
# go-cache threat-model audit
2+
3+
Audit-by: Cerberus (via codex)
4+
Repo: dappco.re/go/cache
5+
Date: 2026-04-25
6+
7+
## 1. Untrusted-key DoS
8+
**Question:** Does every public method (Set, Get, Delete, SetWithTTL, SetBinary, SetBinaryWithTTL, GetBinary, DeleteMany, Clear, ClearScope, Path, Scoped, OnInvalidate, Invalidate) reach `ensureSafeKey` before touching the filesystem? Are there backstops on key length, ScopedCache prefix escape, glob-pattern blowup?
9+
**Finding:** YES - cache keys are bounded and public key-taking methods route through `Path`/`entryPaths` before filesystem access, and scoped prefixes are hashed plus revalidated. However, `Invalidate` accepted callback-returned glob patterns without a length backstop before `keysByPattern` listed and matched all cache keys.
10+
**Severity:** medium
11+
**Repro test:** TestCache_Invalidate_UntrustedPatternLength_Bad
12+
**Fix:** validate invalidation patterns with a fixed byte limit before listing cache entries.
13+
14+
## 2. Path traversal
15+
**Question:** What bytes does `hasPathDangerousBytes` cover (null byte, .., leading / or ~, control chars, URL-encoded %2e%2e)? Does symlink-following escape baseDir? Is `ensureSafeResponseBodyPath` rigour applied to JSON entry paths too?
16+
**Finding:** YES - `ensureSafeKey` rejects empty keys, `..` segments, leading `/` via empty segments, backslashes, null/control bytes, and overlong keys; URL-encoded `%2e%2e` remains a literal safe filename segment. `ensureSafeResponseBodyPath` is applied to cached HTTP response metadata on read and write. The gap was local symlink following: a symlinked directory or file already under `baseDir` could redirect an otherwise safe key outside the cache root.
17+
**Severity:** high
18+
**Repro test:** TestCache_Path_PathTraversalSymlink_Bad
19+
**Fix:** reject existing symlink components from the cache root through the resolved cache path before returning paths for filesystem use.
20+
21+
## 3. Eviction / TOCTOU
22+
**Question:** Is `Cache.mu` (RWMutex) discipline correct? Does `Invalidate` walk the `invalidation` map race-cleanly while `OnInvalidate` may register more? On TTL expiry, do concurrent readers race?
23+
**Finding:** TBD
24+
**Severity:** TBD
25+
**Repro test:** TBD
26+
**Fix:** TBD

0 commit comments

Comments
 (0)