Skip to content

Fix #291: [milestone Milestone 12] internal/symbolic/taint — Taint analysis integration: track data flow from u... - #296

Open
telleroutlook wants to merge 1 commit into
mainfrom
claude/issue-291
Open

Fix #291: [milestone Milestone 12] internal/symbolic/taint — Taint analysis integration: track data flow from u...#296
telleroutlook wants to merge 1 commit into
mainfrom
claude/issue-291

Conversation

@telleroutlook

Copy link
Copy Markdown
Contributor

Fixes #291

Generated by claude-bot-go worker.

…analysis integration: track data flow from u...
@telleroutlook

Copy link
Copy Markdown
Contributor Author

Summary

The taint analyzer can incorrectly report a sink as safe because it does not tie the SMT policy query to the tainted sink or source data.

Severity

high

Blocking findings

  1. internal/symbolic/taint/taint.go:274 The generated violation query contains only execution assumptions and the negated policy; it never encodes the taint relationship or sink reachability, so an unsatisfiable policy can mark any tainted sink safe regardless of whether the sink is actually permitted.

Verdict

❌ Rejected

Merge risk

high

Audit

  • model: opus
  • effort: high
  • tokens: input=0, output=0
{
  "approved": false,
  "severity": "high",
  "summary": "The taint analyzer can incorrectly report a sink as safe because it does not tie the SMT policy query to the tainted sink or source data.",
  "findings": [
    {
      "file": "internal/symbolic/taint/taint.go",
      "line": 274,
      "issue": "The generated violation query contains only execution assumptions and the negated policy; it never encodes the taint relationship or sink reachability, so an unsatisfiable policy can mark any tainted sink safe regardless of whether the sink is actually permitted.",
      "kind": "blocker"
    }
  ],
  "merge_risk": "high",
  "Model": "opus",
  "model": "codex",
  "effort": "high",
  "tokens": {
    "input_tokens": 0,
    "output_tokens": 0
  },
  "same_verdict_count": 1
}

@telleroutlook

Copy link
Copy Markdown
Contributor Author

Summary

The taint analyzer can reject valid Boolean, Real, string, and bit-vector policies because it declares every inferred symbol as Int.

Severity

high

Blocking findings

  1. internal/symbolic/taint/taint.go:354 inferredModel assigns the Int sort to every identifier not explicitly supplied, so valid constraints such as (= enabled true) or (> ratio 0.5) produce invalid SMT declarations and fail analysis instead of returning a policy decision.

Verdict

❌ Rejected

Merge risk

high

Audit

  • model: opus
  • effort: high
  • tokens: input=0, output=0
{
  "approved": false,
  "severity": "high",
  "summary": "The taint analyzer can reject valid Boolean, Real, string, and bit-vector policies because it declares every inferred symbol as Int.",
  "findings": [
    {
      "file": "internal/symbolic/taint/taint.go",
      "line": 354,
      "issue": "inferredModel assigns the Int sort to every identifier not explicitly supplied, so valid constraints such as `(= enabled true)` or `(\u003e ratio 0.5)` produce invalid SMT declarations and fail analysis instead of returning a policy decision.",
      "kind": "blocker"
    }
  ],
  "merge_risk": "high",
  "Model": "opus",
  "model": "codex",
  "effort": "high",
  "tokens": {
    "input_tokens": 0,
    "output_tokens": 0
  },
  "same_verdict_count": 2
}

@telleroutlook

Copy link
Copy Markdown
Contributor Author

Summary

The endpoint has merge-blocking correctness and resource-safety issues in solver request handling.

Severity

high

Blocking findings

  1. internal/symbolic/taint/taint.go:354 inferredModel declares every identifier found by a regex as an Int, including SMT built-ins and non-Int terms such as select, store, and Boolean symbols; valid policies can therefore fail with sort or duplicate-declaration errors instead of producing a taint decision.
  2. internal/symbolic/taint/taint.go:428 TimeoutMs is accepted without an upper bound, allowing callers to request arbitrarily long solver execution and retain Z3 subprocess/server resources for an unbounded duration.

Verdict

❌ Rejected

Merge risk

high

Audit

  • model: opus
  • effort: high
  • tokens: input=0, output=0
{
  "approved": false,
  "severity": "high",
  "summary": "The endpoint has merge-blocking correctness and resource-safety issues in solver request handling.",
  "findings": [
    {
      "file": "internal/symbolic/taint/taint.go",
      "line": 354,
      "issue": "inferredModel declares every identifier found by a regex as an Int, including SMT built-ins and non-Int terms such as select, store, and Boolean symbols; valid policies can therefore fail with sort or duplicate-declaration errors instead of producing a taint decision.",
      "kind": "blocker"
    },
    {
      "file": "internal/symbolic/taint/taint.go",
      "line": 428,
      "issue": "TimeoutMs is accepted without an upper bound, allowing callers to request arbitrarily long solver execution and retain Z3 subprocess/server resources for an unbounded duration.",
      "kind": "blocker"
    }
  ],
  "merge_risk": "high",
  "Model": "opus",
  "model": "codex",
  "effort": "high",
  "tokens": {
    "input_tokens": 0,
    "output_tokens": 0
  },
  "same_verdict_count": 3
}

@telleroutlook

Copy link
Copy Markdown
Contributor Author

Summary

The taint analysis can incorrectly mark unsafe flows as safe because solver errors and unknown outcomes are not consistently handled as merge-blocking security failures.

Severity

high

Blocking findings

  1. internal/symbolic/taint/taint.go:243 A solver result with any Sat value other than exactly "unsat" is treated as an unsafe finding, but the overall analysis returns an error immediately for solver errors. This makes endpoint behavior fail closed only at the request level and prevents returning per-sink unsafe decisions; callers cannot distinguish analysis failure from a proven policy violation, weakening the stated result contract.
  2. internal/symbolic/taint/taint.go:342 The handler accepts a JSON null payload or an empty object as a valid request and returns HTTP 200 with safe=true and zero checked sinks. Invalid or incomplete taint analyses can therefore be reported as safe instead of being rejected, creating a security-sensitive fail-open path.
  3. internal/symbolic/taint/taint.go:220 Sensitive sinks that are tainted but have no policy are marked unsafe, but sinks with empty or whitespace-only names are silently skipped. This can hide malformed security-sensitive operation declarations and produce a safe result without reporting that the requested sink was not analyzed.

Verdict

❌ Rejected

Merge risk

high

Audit

  • model: opus
  • effort: high
  • tokens: input=0, output=0
{
  "approved": false,
  "severity": "high",
  "summary": "The taint analysis can incorrectly mark unsafe flows as safe because solver errors and unknown outcomes are not consistently handled as merge-blocking security failures.",
  "findings": [
    {
      "file": "internal/symbolic/taint/taint.go",
      "line": 243,
      "issue": "A solver result with any Sat value other than exactly \"unsat\" is treated as an unsafe finding, but the overall analysis returns an error immediately for solver errors. This makes endpoint behavior fail closed only at the request level and prevents returning per-sink unsafe decisions; callers cannot distinguish analysis failure from a proven policy violation, weakening the stated result contract.",
      "kind": "blocker"
    },
    {
      "file": "internal/symbolic/taint/taint.go",
      "line": 342,
      "issue": "The handler accepts a JSON null payload or an empty object as a valid request and returns HTTP 200 with safe=true and zero checked sinks. Invalid or incomplete taint analyses can therefore be reported as safe instead of being rejected, creating a security-sensitive fail-open path.",
      "kind": "blocker"
    },
    {
      "file": "internal/symbolic/taint/taint.go",
      "line": 220,
      "issue": "Sensitive sinks that are tainted but have no policy are marked unsafe, but sinks with empty or whitespace-only names are silently skipped. This can hide malformed security-sensitive operation declarations and produce a safe result without reporting that the requested sink was not analyzed.",
      "kind": "suggestion"
    }
  ],
  "merge_risk": "high",
  "Model": "opus",
  "model": "codex",
  "effort": "high",
  "tokens": {
    "input_tokens": 0,
    "output_tokens": 0
  },
  "same_verdict_count": 4
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[milestone Milestone 12] internal/symbolic/taint — Taint analysis integration: track data flow from u...

1 participant