From 2c5385e1cfc4ee29ff85fc85e0df5fb172695898 Mon Sep 17 00:00:00 2001 From: Snider Date: Wed, 15 Apr 2026 20:43:48 +0100 Subject: [PATCH] Harden GUI storage and manifest paths --- pkg/display/display.go | 12 ++++++ pkg/display/hlcrf.go | 15 ++++++-- pkg/display/hlcrf_test.go | 27 ++++++++++++-- pkg/display/manifest.go | 32 +++++++++++++--- pkg/display/manifest_test.go | 13 +++++++ pkg/display/preload.go | 22 ++++++++--- pkg/display/storage.go | 71 +++++++++++++++++++++++++++++++++--- pkg/display/storage_test.go | 18 +++++++++ 8 files changed, 186 insertions(+), 24 deletions(-) diff --git a/pkg/display/display.go b/pkg/display/display.go index 1768e476..0af676dd 100644 --- a/pkg/display/display.go +++ b/pkg/display/display.go @@ -129,6 +129,18 @@ func (s *Service) OnStartup(_ context.Context) core.Result { } return core.Result{Value: map[string]string{"origin": origin, "bucket": bucket, "key": key}, OK: true} }) + s.Core().Action("display.storage.delete", func(_ context.Context, opts core.Options) core.Result { + origin := opts.String("origin") + bucket := opts.String("bucket") + key := opts.String("key") + if s.storage == nil { + return core.Result{Value: coreerr.E("display.storage.delete", "storage registry unavailable", nil), OK: false} + } + if !s.storage.Delete(origin, bucket, key) { + return core.Result{Value: coreerr.E("display.storage.delete", "invalid storage entry", nil), OK: false} + } + return core.Result{Value: map[string]string{"origin": origin, "bucket": bucket, "key": key}, OK: true} + }) s.Core().Action("display.storage.search", func(_ context.Context, opts core.Options) core.Result { return core.Result{Value: s.searchAllStorage(opts.String("q")), OK: true} }) diff --git a/pkg/display/hlcrf.go b/pkg/display/hlcrf.go index 4cea3ce7..b6a4b152 100644 --- a/pkg/display/hlcrf.go +++ b/pkg/display/hlcrf.go @@ -9,16 +9,23 @@ import ( var hlcrfSlotPattern = regexp.MustCompile(`\{\{\s*slot\s+"([^"]+)"\s*\}\}`) -func (s *Service) buildHLCRFComponents(pageURL string) string { +func (s *Service) buildHLCRFComponents(pageURL string) (string, error) { loaded, err := s.loadManifestForOrigin(pageURL) if err != nil || loaded == nil { - return "" + if err != nil && strings.Contains(err.Error(), "view manifest not found") { + return "", nil + } + return "", err } var scripts []string for _, component := range loaded.Manifest.HLCRF { templateBody := strings.TrimSpace(component.Template) if templateBody == "" && strings.TrimSpace(component.Name) != "" { - body, readErr := os.ReadFile(filepath.Join(loaded.BaseDir, component.Name)) + resolvedPath, pathErr := safeManifestRelativePath(loaded.BaseDir, component.Name, "hlcrf component path") + if pathErr != nil { + return "", pathErr + } + body, readErr := os.ReadFile(resolvedPath) if readErr == nil { templateBody = string(body) } @@ -32,7 +39,7 @@ func (s *Service) buildHLCRFComponents(pageURL string) string { } scripts = append(scripts, renderHLCRFComponent(tag, templateBody)) } - return strings.Join(scripts, "\n") + return strings.Join(scripts, "\n"), nil } func renderHLCRFComponent(tag, templateBody string) string { diff --git a/pkg/display/hlcrf_test.go b/pkg/display/hlcrf_test.go index a41ee538..58aba6ce 100644 --- a/pkg/display/hlcrf_test.go +++ b/pkg/display/hlcrf_test.go @@ -36,8 +36,9 @@ func TestHLCRF_BuildHLCRFComponents_Good(t *testing.T) { svc := &Service{} - script := svc.buildHLCRFComponents(filepath.Join(root, "index.html")) + script, err := svc.buildHLCRFComponents(filepath.Join(root, "index.html")) + require.NoError(t, err) require.NotEmpty(t, script) assert.Contains(t, script, "customElements.define") assert.Contains(t, script, "article>Card") @@ -56,8 +57,9 @@ func TestHLCRF_CompileHLCRFTemplate_Good(t *testing.T) { func TestHLCRF_BuildHLCRFComponents_Bad(t *testing.T) { svc := &Service{} - script := svc.buildHLCRFComponents(filepath.Join(t.TempDir(), "missing.html")) + script, err := svc.buildHLCRFComponents(filepath.Join(t.TempDir(), "missing.html")) + require.NoError(t, err) assert.Empty(t, script) } @@ -73,9 +75,28 @@ func TestHLCRF_BuildHLCRFComponents_Ugly(t *testing.T) { svc := &Service{} - script := svc.buildHLCRFComponents(filepath.Join(root, "index.html")) + script, err := svc.buildHLCRFComponents(filepath.Join(root, "index.html")) + require.NoError(t, err) require.NotEmpty(t, script) assert.Contains(t, script, "Fallback") assert.NotContains(t, script, "missing.html") } + +func TestHLCRF_BuildHLCRFComponents_RejectsTraversal(t *testing.T) { + root := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(root, ".core"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(root, "index.html"), []byte(""), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(root, "outside.html"), []byte("Outside"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(root, ".core", "view.yaml"), []byte(strings.Join([]string{ + "hlcrf:", + " - name: ../outside.html", + }, "\n")), 0o644)) + + svc := &Service{} + + script, err := svc.buildHLCRFComponents(filepath.Join(root, "index.html")) + + require.Error(t, err) + assert.Empty(t, script) +} diff --git a/pkg/display/manifest.go b/pkg/display/manifest.go index 23c5a234..e3ef60ab 100644 --- a/pkg/display/manifest.go +++ b/pkg/display/manifest.go @@ -83,18 +83,26 @@ func manifestBaseDir(manifestPath string) string { } func safeManifestPreloadPath(baseDir, preloadPath string) (string, error) { - trimmed := strings.TrimSpace(preloadPath) + return safeManifestRelativePath(baseDir, preloadPath, "preload path") +} + +func safeManifestRelativePath(baseDir, relativePath, label string) (string, error) { + trimmed := strings.TrimSpace(relativePath) if trimmed == "" { - return "", errors.New("preload path is empty") + return "", errors.New(label + " is empty") } if filepath.IsAbs(trimmed) { - return "", errors.New("preload path must be relative") + return "", errors.New(label + " must be relative") } baseAbs, err := filepath.Abs(baseDir) if err != nil { return "", err } + baseResolved, err := filepath.EvalSymlinks(baseAbs) + if err != nil { + return "", err + } candidateAbs, err := filepath.Abs(filepath.Join(baseAbs, trimmed)) if err != nil { return "", err @@ -104,9 +112,23 @@ func safeManifestPreloadPath(baseDir, preloadPath string) (string, error) { return "", err } if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { - return "", errors.New("preload path escapes manifest directory") + return "", errors.New(label + " escapes manifest directory") } - return candidateAbs, nil + if _, statErr := os.Stat(candidateAbs); statErr != nil { + return candidateAbs, nil + } + candidateResolved, err := filepath.EvalSymlinks(candidateAbs) + if err != nil { + return "", err + } + rel, err = filepath.Rel(baseResolved, candidateResolved) + if err != nil { + return "", err + } + if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return "", errors.New(label + " escapes manifest directory") + } + return candidateResolved, nil } func discoverManifestPath(pageURL string) (string, error) { diff --git a/pkg/display/manifest_test.go b/pkg/display/manifest_test.go index 92024b52..852f5ca5 100644 --- a/pkg/display/manifest_test.go +++ b/pkg/display/manifest_test.go @@ -63,6 +63,19 @@ func TestManifest_SafeManifestPreloadPath_Ugly(t *testing.T) { assert.Contains(t, err.Error(), "escapes") } +func TestManifest_SafeManifestPreloadPath_RejectsSymlinkEscape(t *testing.T) { + root := t.TempDir() + outside := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(outside, "preload.js"), []byte("globalThis.__outside = true;"), 0o644)) + require.NoError(t, os.MkdirAll(filepath.Join(root, "assets"), 0o755)) + require.NoError(t, os.Symlink(outside, filepath.Join(root, "assets", "linked"))) + + _, err := safeManifestPreloadPath(filepath.Join(root, "assets"), filepath.Join("linked", "preload.js")) + + require.Error(t, err) + assert.Contains(t, err.Error(), "escapes") +} + func TestManifest_DiscoverManifestPath_Good(t *testing.T) { root := t.TempDir() require.NoError(t, os.MkdirAll(filepath.Join(root, ".core"), 0o755)) diff --git a/pkg/display/preload.go b/pkg/display/preload.go index 2d06b4a2..eea6cbf2 100644 --- a/pkg/display/preload.go +++ b/pkg/display/preload.go @@ -41,7 +41,11 @@ func (s *Service) BuildPreloadScript(pageURL string) (string, error) { parts := []string{ s.injectStoragePolyfills(pageURL, storageBootstrap, trustedOrigin), s.injectCoreMLShim(trustedOrigin), - s.buildHLCRFComponents(pageURL), + } + if hlcrfComponents, err := s.buildHLCRFComponents(pageURL); err != nil { + return "", err + } else if strings.TrimSpace(hlcrfComponents) != "" { + parts = append(parts, hlcrfComponents) } if trustedOrigin { parts = append(parts, @@ -251,11 +255,17 @@ func (s *Service) injectStoragePolyfills(pageOrigin string, bootstrap map[string } __coreBridge.invoke('display.storage.set', { origin: __coreOrigin, bucket, key, value }).catch(() => undefined); }; + const persistDelete = (bucket, key) => { + if (!__coreCanInvoke) { + return; + } + __coreBridge.invoke('display.storage.delete', { origin: __coreOrigin, bucket, key }).catch(() => undefined); + }; const createStorage = (bucketName, bucket) => ({ getItem(key) { return Object.prototype.hasOwnProperty.call(bucket, key) ? String(bucket[key]) : null; }, setItem(key, value) { bucket[key] = String(value); persist(bucketName, key, bucket[key]); }, - removeItem(key) { delete bucket[key]; persist(bucketName, key, ''); }, - clear() { Object.keys(bucket).forEach((key) => { delete bucket[key]; persist(bucketName, key, ''); }); }, + removeItem(key) { delete bucket[key]; persistDelete(bucketName, key); }, + clear() { Object.keys(bucket).forEach((key) => { delete bucket[key]; persistDelete(bucketName, key); }); }, key(index) { return Object.keys(bucket)[index] ?? null; }, get length() { return Object.keys(bucket).length; } }); @@ -360,7 +370,7 @@ func (s *Service) injectStoragePolyfills(pageOrigin string, bootstrap map[string }); if (cookieIsExpired(record)) { delete __scope.cookies[name]; - persist('cookies', name, ''); + persistDelete('cookies', name); return; } __scope.cookies[name] = record; @@ -393,7 +403,7 @@ func (s *Service) injectStoragePolyfills(pageOrigin string, bootstrap map[string if (database.stores?.[storeName]) { delete database.stores[storeName][key]; } - persist('indexeddb:' + databaseName, storeName + ':' + key, ''); + persistDelete('indexeddb:' + databaseName, storeName + ':' + key); return createIDBRequest(undefined); }, clear() { @@ -439,7 +449,7 @@ func (s *Service) injectStoragePolyfills(pageOrigin string, bootstrap map[string async delete(request) { const key = typeof request === 'string' ? request : request?.url; delete bucket[key]; - persist('cache:' + name, key, ''); + persistDelete('cache:' + name, key); return true; }, async keys() { diff --git a/pkg/display/storage.go b/pkg/display/storage.go index 0ba3f004..875c673c 100644 --- a/pkg/display/storage.go +++ b/pkg/display/storage.go @@ -15,11 +15,13 @@ import ( ) const ( - maxStorageOriginBytes = 512 - maxStorageBucketBytes = 128 - maxStorageKeyBytes = 1024 - maxStorageValueBytes = 1 << 20 - maxStorageSearchResults = 200 + maxStorageOriginBytes = 512 + maxStorageBucketBytes = 128 + maxStorageKeyBytes = 1024 + maxStorageValueBytes = 1 << 20 + maxStorageEntriesPerOrigin = 1024 + maxStorageBytesPerOrigin = 16 << 20 + maxStorageSearchResults = 200 ) type StorageEntry struct { @@ -169,6 +171,15 @@ func (r *StorageRegistry) Set(origin, bucket, key, value string) bool { key = strings.TrimSpace(key) r.mu.Lock() defer r.mu.Unlock() + composite := makeStorageEntryKey(origin, bucket, key) + if !r.withinOriginQuotaLocked(origin, composite, StorageEntry{ + Origin: origin, + Bucket: bucket, + Key: key, + Value: value, + }) { + return false + } entry := StorageEntry{ Origin: origin, Bucket: bucket, @@ -176,7 +187,6 @@ func (r *StorageRegistry) Set(origin, bucket, key, value string) bool { Value: value, UpdatedAt: time.Now(), } - composite := makeStorageEntryKey(origin, bucket, key) r.entries[composite] = entry if r.store != nil { if err := r.store.Set("storage", storageCompositeKey(origin, bucket, key), core.JSONMarshalString(entry)); err != nil { @@ -186,6 +196,27 @@ func (r *StorageRegistry) Set(origin, bucket, key, value string) bool { return true } +func (r *StorageRegistry) Delete(origin, bucket, key string) bool { + if !validStorageField(origin, maxStorageOriginBytes) || + !validStorageField(bucket, maxStorageBucketBytes) || + !validStorageField(key, maxStorageKeyBytes) { + return false + } + origin = strings.TrimSpace(origin) + bucket = strings.TrimSpace(bucket) + key = strings.TrimSpace(key) + r.mu.Lock() + defer r.mu.Unlock() + composite := makeStorageEntryKey(origin, bucket, key) + delete(r.entries, composite) + if r.store != nil { + if err := r.store.Delete("storage", storageCompositeKey(origin, bucket, key)); err != nil { + return false + } + } + return true +} + func (r *StorageRegistry) Get(origin, bucket, key string) (StorageEntry, bool) { r.mu.RLock() defer r.mu.RUnlock() @@ -262,6 +293,34 @@ func (r *StorageRegistry) Snapshot(pageURL string) map[string]map[string]string return snapshot } +func (r *StorageRegistry) withinOriginQuotaLocked(origin, ignoreComposite string, candidate StorageEntry) bool { + entries := 0 + bytes := 0 + for composite, entry := range r.entries { + if !strings.EqualFold(entry.Origin, origin) { + continue + } + if composite == ignoreComposite { + continue + } + entries++ + bytes += storageEntrySizeBytes(entry) + } + entries++ + bytes += storageEntrySizeBytes(candidate) + if entries > maxStorageEntriesPerOrigin { + return false + } + if bytes > maxStorageBytesPerOrigin { + return false + } + return true +} + +func storageEntrySizeBytes(entry StorageEntry) int { + return len(entry.Origin) + len(entry.Bucket) + len(entry.Key) + len(entry.Value) +} + func (r *StorageRegistry) Close() error { if r == nil || r.store == nil { return nil diff --git a/pkg/display/storage_test.go b/pkg/display/storage_test.go index 6df22a22..b39a7dda 100644 --- a/pkg/display/storage_test.go +++ b/pkg/display/storage_test.go @@ -1,6 +1,7 @@ package display import ( + "fmt" "strings" "testing" "time" @@ -117,6 +118,23 @@ func TestStorageRegistry_Set_Bad(t *testing.T) { assert.False(t, r.Set("core://settings", "localStorage", "theme", strings.Repeat("x", maxStorageValueBytes+1))) } +func TestStorageRegistry_Delete_Good(t *testing.T) { + r := NewStorageRegistry() + r.Set("core://settings", "localStorage", "theme", "dark") + + assert.True(t, r.Delete("core://settings", "localStorage", "theme")) + _, ok := r.Get("core://settings", "localStorage", "theme") + assert.False(t, ok) +} + +func TestStorageRegistry_Set_RejectsQuotaOverflow(t *testing.T) { + r := NewStorageRegistry() + for i := 0; i < maxStorageEntriesPerOrigin; i++ { + require.True(t, r.Set("core://settings", "localStorage", fmt.Sprintf("key-%d", i), "v")) + } + assert.False(t, r.Set("core://settings", "localStorage", "overflow", "v")) +} + func TestStorage_StorageOriginForPageURL_Good(t *testing.T) { assert.Equal(t, "https://app.example.com", storageOriginForPageURL("https://app.example.com/path?q=1")) assert.Equal(t, "core://settings", storageOriginForPageURL("core://settings/view"))