Skip to content

Commit 68078c4

Browse files
committed
fix: next 5 exploitable vulnerabilities (base64 wrapping, browser title/element wrapping + cap, config size cap, resource resolver symlink Lstat)
1 parent 8dbba3d commit 68078c4

10 files changed

Lines changed: 179 additions & 7 deletions

File tree

AGENTS.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ System prompt is loaded by priority: `--system` flag > `~/.odek/IDENTITY.md` > c
9191
### Security Architecture
9292
Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECURITY.md](docs/SECURITY.md).
9393

94-
- **Untrusted-content wrapper** (`cmd/odek/untrusted.go`) — every tool whose output sources from outside the trust boundary (`browser`, `read_file`, `shell`, `search_files`, `multi_grep`, `transcribe`, `head_tail`, `diff`, `tr`, `sort`, `json_query`, `batch_patch`, `glob`, `file_info`, `tree`, `session_search`, `@-resources`, `--ctx` files, any MCP tool) wraps results in `<untrusted_content_<nonce> source="...">…</untrusted_content_<nonce>>`. Per-call nonce defeats wrapper-escape via literal close tag.
94+
- **Untrusted-content wrapper** (`cmd/odek/untrusted.go`) — every tool whose output sources from outside the trust boundary (`browser`, `read_file`, `shell`, `search_files`, `multi_grep`, `transcribe`, `head_tail`, `diff`, `tr`, `sort`, `json_query`, `batch_patch`, `glob`, `file_info`, `tree`, `base64` file mode, `session_search`, `@-resources`, `--ctx` files, any MCP tool) wraps results in `<untrusted_content_<nonce> source="...">…</untrusted_content_<nonce>>`. Browser page title and interactive-element text are wrapped in addition to the main content. Per-call nonce defeats wrapper-escape via literal close tag.
9595
- **Audit log** (`cmd/odek/audit.go` + `internal/session/audit.go`) — every `wrapUntrusted` call records source + content-hash + turn into `<sessions>/audit/<id>.json`. After each turn a divergence heuristic flags `suspicious_divergence=true` when the agent ingested untrusted content AND its tool calls referenced resources the user did not mention. Inspect with `odek audit <session-id>` / `odek audit --list`.
9696
- **Memory taint** (`internal/memory/provenance.go`) — `EpisodeProvenance` tracks Untrusted/Sources/UserApproved. Tainted episodes are stored but `Search()` filters them out, so a one-shot injection cannot persist via the episode pipeline. User must explicitly promote.
9797
- **Skill provenance gate** (`internal/skills/loader.go` + `cache.go`) — `Skill.Provenance{Untrusted, Sources, NeedsReview}`. NeedsReview skills pin to Lazy regardless of `auto_load`. `odek skill promote <name>` clears the flag after user review.
@@ -102,6 +102,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
102102
- **WS Origin allowlist** (`cmd/odek/serve.go::checkLocalOrigin`) — rejects non-localhost upgrades. Closes CSRF-on-localhost.
103103
- **REST API CSRF protection** (`cmd/odek/serve.go::requireLocalOrigin`) — state-changing HTTP endpoints (POST/PUT/PATCH/DELETE) require a localhost origin or no Origin header, and static responses set `X-Frame-Options: DENY` + `Content-Security-Policy: frame-ancestors 'none'` to block clickjacking.
104104
- **Browser history cap** (`cmd/odek/browser_tool.go`) — navigation history is capped at 50 snapshots to prevent memory DoS from repeated `browser_navigate` calls.
105+
- **Browser element cap** (`cmd/odek/browser_tool.go`) — the number of interactive elements extracted per page is capped at 500 so a hostile page cannot OOM the agent with thousands of links or buttons.
105106
- **Search result bounds** (`cmd/odek/file_tool.go`, `cmd/odek/perf_tools.go`) — `search_files` and `multi_grep` enforce a max match limit (500) and a total returned-content cap (1 MiB) to avoid unbounded result JSON.
106107
- **Perf-tool file-size cap** (`cmd/odek/perf_tools.go`) — `diff`, `base64`, `tr`, `sort`, `json_query`, and `batch_patch` reject files larger than 10 MiB to avoid loading multi-gigabyte files into memory.
107108
- **Shell output cap** (`cmd/odek/shell.go`, `cmd/odek/perf_tools.go`) — `shell` and `parallel_shell` cap captured stdout/stderr at 1 MiB per stream to prevent memory DoS from commands that dump huge files.
@@ -113,7 +114,9 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
113114
- **Sub-agent task-file cap** (`cmd/odek/subagent.go`) — `odek subagent --task <file>` rejects task files larger than 10 MiB before loading them into memory.
114115
- **session_search hardening** (`cmd/odek/session_search_tool.go`) — the `get` action returns at most the 100 most recent messages and wraps each message content, task, and buffer entry as untrusted; `list`/`search`/`find` also wrap session tasks.
115116
- **@-resource / --ctx prompt wrapping** (`cmd/odek/refs.go`, `cmd/odek/serve.go`) — content resolved from `@file` references and `--ctx` files is wrapped as untrusted before being inserted into the prompt.
117+
- **Config file size cap** (`internal/config/loader.go`) — `~/.odek/config.json` and `./odek.json` are rejected if larger than 5 MiB to prevent OOM from a malicious or broken config at startup.
116118
- **Resource resolver size cap** (`internal/resource/resource.go`) — `@-resource` file loads are capped at 1 MiB to prevent OOM from `@hugefile` references.
119+
- **Resource resolver symlink hardening** (`internal/resource/resource.go`) — `FileResolver.Search` uses `os.Lstat` (not `os.Stat`) for search-result metadata, so symlinks cannot leak the size of arbitrary targets outside the workspace.
117120
- **Sub-agent summary cap** (`cmd/odek/subagent_tool.go`) — each sub-agent result included in the `delegate_tasks` summary is truncated to 100 KiB to prevent memory DoS.
118121
- **Tree path wrapping** (`cmd/odek/perf_tools.go`) — the `tree` tool wraps every filesystem-derived path as untrusted content.
119122
- **head_tail output cap** (`cmd/odek/perf_tools.go`) — `head_tail` truncates returned lines so total content stays within 1 MiB, preventing multi-file/multi-line memory DoS.

cmd/odek/browser_tool.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,11 @@ type browserSnapshot struct {
4949
// prevent memory DoS from repeated navigate actions.
5050
const maxBrowserHistory = 50
5151

52+
// maxBrowserElements caps the number of interactive elements extracted from a
53+
// page to prevent a hostile page from OOMing the agent with thousands of links
54+
// or buttons.
55+
const maxBrowserElements = 500
56+
5257
// browserState holds the shared state for one browser session.
5358
type browserState struct {
5459
mu sync.Mutex
@@ -379,6 +384,9 @@ func parseHTML(html, pageURL string, status int) browserSnapshot {
379384

380385
// Extract links
381386
for _, m := range reLink.FindAllStringSubmatch(html, -1) {
387+
if len(elements) >= maxBrowserElements {
388+
break
389+
}
382390
href := strings.TrimSpace(m[1])
383391
text := strings.TrimSpace(m[2])
384392
if href == "" || text == "" || href == "#" || strings.HasPrefix(href, "javascript:") {
@@ -404,6 +412,9 @@ func parseHTML(html, pageURL string, status int) browserSnapshot {
404412

405413
// Extract buttons and inputs
406414
for _, m := range reButton.FindAllStringSubmatch(html, -1) {
415+
if len(elements) >= maxBrowserElements {
416+
break
417+
}
407418
text := strings.TrimSpace(m[1])
408419
if text == "" {
409420
text = "button"
@@ -419,6 +430,9 @@ func parseHTML(html, pageURL string, status int) browserSnapshot {
419430
}
420431

421432
for _, m := range reInput.FindAllStringSubmatch(html, -1) {
433+
if len(elements) >= maxBrowserElements {
434+
break
435+
}
422436
tag := m[0]
423437
text := ""
424438
if vm := reInputVal.FindStringSubmatch(tag); len(vm) > 1 {
@@ -442,6 +456,12 @@ func parseHTML(html, pageURL string, status int) browserSnapshot {
442456
snap.Content = strings.Join(contentParts, "\n")
443457
snap.Elements = elements
444458

459+
// Title and element text come from the page — wrap them as untrusted content.
460+
snap.Title = wrapUntrusted(pageURL, snap.Title)
461+
for i := range snap.Elements {
462+
snap.Elements[i].Text = wrapUntrusted(pageURL, snap.Elements[i].Text)
463+
}
464+
445465
return snap
446466
}
447467

cmd/odek/browser_tool_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ func TestBrowser_Navigate(t *testing.T) {
3434
if r.Error != "" {
3535
t.Fatalf("navigate error: %s", r.Error)
3636
}
37-
if r.Title != "Test Page" {
38-
t.Errorf("title = %q, want %q", r.Title, "Test Page")
37+
if unwrapUntrusted(r.Title) != "Test Page" {
38+
t.Errorf("title = %q, want %q", unwrapUntrusted(r.Title), "Test Page")
3939
}
4040
if !strings.Contains(r.Content, "Hello World") {
4141
t.Errorf("content missing 'Hello World': %q", r.Content)

cmd/odek/next_security_vulnerabilities_test.go

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -901,6 +901,80 @@ func TestSkillLoader_CapsFileSize(t *testing.T) {
901901
}
902902
}
903903

904+
// ── 26. base64 must wrap file-mode encoded output as untrusted ───────────
905+
906+
func TestBase64_WrapsFileEncodedContent(t *testing.T) {
907+
dir := t.TempDir()
908+
path := filepath.Join(dir, "secret.txt")
909+
os.WriteFile(path, []byte("sensitive data"), 0644)
910+
911+
tool := &base64Tool{dangerousConfig: danger.DangerousConfig{}}
912+
result := callJSON(t, tool, fmt.Sprintf(`{"path":%q}`, path))
913+
var r struct {
914+
Encoded string `json:"encoded"`
915+
Size int `json:"size"`
916+
}
917+
mustUnmarshal(t, result, &r)
918+
if r.Size == 0 {
919+
t.Fatal("expected size > 0")
920+
}
921+
if !strings.HasPrefix(r.Encoded, "<untrusted_content_") {
922+
t.Fatalf("base64 file output should be wrapped in untrusted_content, got: %q", r.Encoded)
923+
}
924+
}
925+
926+
// ── 27. browser must wrap page title / element text ──────────────────────
927+
928+
func TestBrowser_WrapsTitleAndElementText(t *testing.T) {
929+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
930+
fmt.Fprint(w, `<html><head><title>Evil Title</title></head><body><a href="/x">click me</a></body></html>`)
931+
}))
932+
defer srv.Close()
933+
934+
tool := newBrowserTool(danger.DangerousConfig{})
935+
result := callJSON(t, tool, fmt.Sprintf(`{"action":"navigate","url":%q}`, srv.URL))
936+
var r struct {
937+
Title string `json:"title"`
938+
Elements []struct {
939+
Text string `json:"text"`
940+
URL string `json:"url"`
941+
} `json:"elements"`
942+
}
943+
mustUnmarshal(t, result, &r)
944+
if !strings.HasPrefix(r.Title, "<untrusted_content_") {
945+
t.Fatalf("browser title should be wrapped, got: %q", r.Title)
946+
}
947+
if len(r.Elements) == 0 {
948+
t.Fatal("expected at least one element")
949+
}
950+
if !strings.HasPrefix(r.Elements[0].Text, "<untrusted_content_") {
951+
t.Fatalf("browser element text should be wrapped, got: %q", r.Elements[0].Text)
952+
}
953+
}
954+
955+
// ── 28. browser must cap the number of interactive elements ──────────────
956+
957+
func TestBrowser_CapsElementCount(t *testing.T) {
958+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
959+
w.Write([]byte("<html><body>"))
960+
for i := 0; i < 1500; i++ {
961+
fmt.Fprintf(w, `<a href="/p%d">link %d</a>`, i, i)
962+
}
963+
w.Write([]byte("</body></html>"))
964+
}))
965+
defer srv.Close()
966+
967+
tool := newBrowserTool(danger.DangerousConfig{})
968+
result := callJSON(t, tool, fmt.Sprintf(`{"action":"navigate","url":%q}`, srv.URL))
969+
var r struct {
970+
Elements []any `json:"elements"`
971+
}
972+
mustUnmarshal(t, result, &r)
973+
if len(r.Elements) > 1000 {
974+
t.Fatalf("browser did not cap element count: got %d", len(r.Elements))
975+
}
976+
}
977+
904978
// ── 25. tree must wrap filesystem-derived paths as untrusted ──────────────
905979

906980
func TestTree_WrapsPaths(t *testing.T) {

cmd/odek/perf_tools.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2146,7 +2146,7 @@ func (t *base64Tool) Call(argsJSON string) (result string, err error) {
21462146
return jsonResult(base64Result{Error: fmt.Sprintf("cannot read %q: %v", args.Path, err)})
21472147
}
21482148
encoded := base64.StdEncoding.EncodeToString(data)
2149-
return jsonResult(base64Result{Encoded: encoded, Size: len(data)})
2149+
return jsonResult(base64Result{Encoded: wrapUntrusted(args.Path, encoded), Size: len(data)})
21502150
}
21512151

21522152
// ═════════════════════════════════════════════════════════════════════════

docs/SECURITY.md

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ Tools that wrap:
5353

5454
| Tool | Source attribute |
5555
|---|---|
56-
| `browser` (navigate / snapshot / back) | the URL |
56+
| `browser` (navigate / snapshot / back) | the URL; page title and interactive-element text are wrapped too |
5757
| `read_file` | the absolute path |
5858
| `search_files`, `multi_grep` | `<path>:<line>` per match |
5959
| `shell` | `$ <command>` |
@@ -63,6 +63,7 @@ Tools that wrap:
6363
| `session_search` | `session_search` (whole result — past sessions may be tainted) |
6464
| `file_info` | `file_info:<path>` (metadata about an external file) |
6565
| `tree` | `tree:<root>` (directory/file names from the filesystem) |
66+
| `base64` (file/path mode) | `base64:<path>` (the encoded bytes are wrapped) |
6667
| any MCP tool | `mcp:<server>:<tool>` |
6768

6869
`session_search` is wrapped because it can surface content from arbitrary past sessions — including sessions that ingested untrusted content. Wrapping its whole output keeps that content from re-entering as trusted instructions and records the retrieval in the audit log, closing a path that otherwise bypassed the memory taint gate (defense 5).
@@ -71,6 +72,8 @@ The MCP wrapper guards a tool's **output**. The server-supplied tool **descripti
7172

7273
The model is instructed (via the default system prompt) to treat the wrapped region as data, not instructions. A model trained on prompt-injection resistance (Claude Sonnet 4.6+ does this well) honours the boundary. Older models or aggressively fine-tuned ones may not.
7374

75+
Two additional boundaries keep filesystem-derived metadata from leaking as "trusted" context. First, the `base64` tool wraps encoded output when reading from a file path, so even transformed filesystem bytes stay inside an untrusted boundary. Second, the `@`-resource resolver (`FileResolver.Search`) uses `os.Lstat` when building search-result metadata, which prevents a symlink inside the workspace from leaking the size (or other `stat` metadata) of an arbitrary target outside it.
76+
7477
### 3. Danger classifier (shell)
7578

7679
The `shell` tool tokenises commands and classifies each into one of 9 risk classes (`safe`, `local_write`, `system_write`, `destructive`, `network_egress`, `code_execution`, `install`, `unknown`, `blocked`). Per-class policy (allow / prompt / deny) is configurable.
@@ -257,6 +260,10 @@ See [CLI.md — Dangerous Operations](CLI.md#dangerous-operations) for the full
257260
}
258261
```
259262

263+
### 15. Configuration file size cap
264+
265+
`~/.odek/config.json` and `./odek.json` are rejected if they exceed 5 MiB. This prevents a malicious, truncated, or accidentally-generated config file from causing an out-of-memory condition at startup.
266+
260267
### YOLO mode
261268

262269
```json

internal/config/loader.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ import (
3030
"github.com/BackendStack21/odek/internal/telegram"
3131
)
3232

33+
// maxConfigFileBytes caps how large a config file may be before it is rejected.
34+
// This prevents a malicious or broken config from OOMing startup.
35+
const maxConfigFileBytes = 5 << 20 // 5 MiB
36+
3337
// ── Types ──────────────────────────────────────────────────────────────
3438

3539
// CLIFlags holds values parsed from the CLI. Zero/nil values mean the
@@ -408,10 +412,18 @@ func loadFile(path string) FileConfig {
408412
if path == "" {
409413
return FileConfig{}
410414
}
411-
data, err := os.ReadFile(path)
415+
info, err := os.Stat(path)
412416
if err != nil {
413417
return FileConfig{} // missing or unreadable = empty
414418
}
419+
if info.Size() > maxConfigFileBytes {
420+
fmt.Fprintf(os.Stderr, "odek: warning: config %s: file exceeds maximum size %d bytes — ignoring file\n", path, maxConfigFileBytes)
421+
return FileConfig{}
422+
}
423+
data, err := os.ReadFile(path)
424+
if err != nil {
425+
return FileConfig{} // unreadable = empty
426+
}
415427
var cfg FileConfig
416428
if err := json.Unmarshal(data, &cfg); err != nil {
417429
fmt.Fprintf(os.Stderr, "odek: warning: config %s: invalid JSON — ignoring file: %v\n", path, err)

internal/config/loader_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package config
33
import (
44
"os"
55
"path/filepath"
6+
"strings"
67
"testing"
78

89
"github.com/BackendStack21/odek/internal/memory"
@@ -1015,3 +1016,17 @@ func TestLoadConfig_EmbeddingOverrides(t *testing.T) {
10151016
t.Errorf("explicit skills timeout = %d, want 7 (respected as-is)", cfg.Skills.Embedding.TimeoutSeconds)
10161017
}
10171018
}
1019+
1020+
// TestLoadFile_CapsSize verifies that config files larger than maxConfigFileBytes
1021+
// are ignored to prevent OOM from a malicious or broken config file.
1022+
func TestLoadFile_CapsSize(t *testing.T) {
1023+
dir := t.TempDir()
1024+
path := filepath.Join(dir, "odek.json")
1025+
if err := os.WriteFile(path, []byte(strings.Repeat("x", maxConfigFileBytes+1)), 0644); err != nil {
1026+
t.Fatal(err)
1027+
}
1028+
cfg := loadFile(path)
1029+
if cfg.Model != "" {
1030+
t.Fatalf("loadFile should reject a huge config file, got Model=%q", cfg.Model)
1031+
}
1032+
}

internal/resource/resource.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,8 @@ func (f *FileResolver) Search(ctx context.Context, query string, limit int) ([]R
229229
break
230230
}
231231
rel, _ := filepath.Rel(f.root, match)
232-
info, err := os.Stat(match)
232+
// Use Lstat so that symlinks do not leak metadata from their targets.
233+
info, err := os.Lstat(match)
233234
if err != nil {
234235
continue
235236
}

internal/resource/resource_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package resource
22

33
import (
4+
"bytes"
45
"context"
56
"fmt"
67
"os"
@@ -649,3 +650,42 @@ func TestSessionResolverLoad_PathTraversal(t *testing.T) {
649650
}
650651
}
651652
}
653+
654+
// ── Bug #30: FileResolver.Search follows symlinks via os.Stat ───────────────
655+
656+
func TestFileResolverSearch_DoesNotFollowSymlinksForMetadata(t *testing.T) {
657+
dir := t.TempDir()
658+
base := filepath.Join(dir, "base")
659+
if err := os.MkdirAll(base, 0755); err != nil {
660+
t.Fatal(err)
661+
}
662+
663+
secret := filepath.Join(dir, "secret.txt")
664+
// 2000 bytes produces "2.0 KB" in Detail if os.Stat follows the symlink.
665+
if err := os.WriteFile(secret, bytes.Repeat([]byte("x"), 2000), 0600); err != nil {
666+
t.Fatal(err)
667+
}
668+
link := filepath.Join(base, "leak.txt")
669+
if err := os.Symlink(secret, link); err != nil {
670+
t.Fatal(err)
671+
}
672+
673+
resolver := NewFileResolver(base)
674+
results, err := resolver.Search(context.Background(), "leak", 10)
675+
if err != nil {
676+
t.Fatalf("Search failed: %v", err)
677+
}
678+
if len(results) != 1 {
679+
t.Fatalf("expected 1 result, got %d", len(results))
680+
}
681+
res := results[0]
682+
// With os.Stat the target's size (2.0 KB) would leak through Detail.
683+
// With os.Lstat we get the symlink's own metadata, never the target size.
684+
if strings.Contains(res.Detail, "2.0") {
685+
t.Errorf("symlink leaked target file size through Detail: %s", res.Detail)
686+
}
687+
// The returned resource reference must point to the symlink inside the base.
688+
if res.ID != "@leak.txt" {
689+
t.Errorf("expected resource ID %q, got %q", "@leak.txt", res.ID)
690+
}
691+
}

0 commit comments

Comments
 (0)