From a813357b4bf5fbb09ca0da603e744764be50f351 Mon Sep 17 00:00:00 2001 From: Adarsh Prashar Date: Sun, 31 May 2026 03:24:14 +0530 Subject: [PATCH] fix: close CodeQL path-injection and integer-overflow alerts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the 4 high-severity code-scanning alerts. - memory Reader (go/path-injection, reader.go): validate the cleaned user-supplied path with filepath.IsLocal — rejects absolute paths, "..", and any escape, and is recognized as a path-traversal sanitizer — backed by the existing within-root prefix check as defense in depth. - config budget (go/incorrect-integer-conversion, config.go): clamp the parsed int64 loop/seconds values to math.MaxInt32 before narrowing to int32, so an out-of-range value can't overflow. Adds a regression test. Fixes #10 Fixes #11 --- internal/config/config.go | 15 +++++++++++++-- internal/config/config_test.go | 20 ++++++++++++++++++++ internal/memory/reader.go | 15 +++++++++++++-- 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 407bfca..24ccba5 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -6,6 +6,7 @@ package config import ( "bufio" "fmt" + "math" "os" "strconv" "strings" @@ -245,15 +246,25 @@ func loadBudget() (BudgetConfig, error) { if loops, err = envInt64("RISKKERNEL_DEFAULT_LOOPS"); err != nil { return b, err } - b.Loops = int32(loops) + b.Loops = clampInt32(loops) var secs int64 if secs, err = envInt64("RISKKERNEL_DEFAULT_SECONDS"); err != nil { return b, err } - b.Seconds = int32(secs) + b.Seconds = clampInt32(secs) return b, nil } +// clampInt32 narrows a non-negative int64 to int32, bounding it at math.MaxInt32 +// so an out-of-range value can't silently overflow on conversion (envInt64 +// already rejects negatives). +func clampInt32(v int64) int32 { + if v > math.MaxInt32 { + return math.MaxInt32 + } + return int32(v) +} + func envInt64(key string) (int64, error) { v := os.Getenv(key) if v == "" { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 3fb668e..4311a48 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1,11 +1,31 @@ package config import ( + "math" "os" "path/filepath" "testing" ) +func TestLoad_BudgetClampsInt32Overflow(t *testing.T) { + withCleanEnv(t) + chdirTemp(t) + // A value beyond int32 must clamp to MaxInt32, not silently overflow/wrap. + t.Setenv("RISKKERNEL_DEFAULT_LOOPS", "5000000000") // > math.MaxInt32 + t.Setenv("RISKKERNEL_DEFAULT_SECONDS", "5000000000") + + cfg, err := Load() + if err != nil { + t.Fatalf("Load: %v", err) + } + if cfg.DefaultBudget.Loops != math.MaxInt32 { + t.Errorf("Loops = %d, want clamp to %d", cfg.DefaultBudget.Loops, int32(math.MaxInt32)) + } + if cfg.DefaultBudget.Seconds != math.MaxInt32 { + t.Errorf("Seconds = %d, want clamp to %d", cfg.DefaultBudget.Seconds, int32(math.MaxInt32)) + } +} + func TestLoad_Defaults(t *testing.T) { withCleanEnv(t) chdirTemp(t) // no .env present diff --git a/internal/memory/reader.go b/internal/memory/reader.go index 47d7340..79e8e67 100644 --- a/internal/memory/reader.go +++ b/internal/memory/reader.go @@ -163,9 +163,20 @@ func (r *Reader) resolveFile(namespace, name string) (string, error) { return r.safeJoin(filepath.Join(namespace, name)) } -// safeJoin joins rel under the root and guarantees the result stays within it. +// safeJoin joins a user-supplied relative path under the root and guarantees the +// result cannot escape it. The cleaned path is validated with filepath.IsLocal — +// which rejects absolute paths, "..", and anything that would escape the base +// (and which static analysis recognizes as a path-traversal sanitizer) — backed +// by a prefix check as defense in depth. func (r *Reader) safeJoin(rel string) (string, error) { - joined := filepath.Clean(filepath.Join(r.root, rel)) + clean := filepath.Clean(rel) + if clean == "." { // the root itself (e.g. an empty namespace) + return r.root, nil + } + if !filepath.IsLocal(clean) { + return "", ErrUnsafePath + } + joined := filepath.Join(r.root, clean) if joined != r.root && !strings.HasPrefix(joined, r.root+string(os.PathSeparator)) { return "", ErrUnsafePath }