Skip to content

[Bug]: Phase 2 — High Priority: Rate Limiting, Request Size, Webhook Auth, Event Dedup, Trust & Symlinks (H-1 through H-10) #31

@initializ-mk

Description

@initializ-mk

Component

forge-core (registry, tools, security, channels, LLM), forge-cli (commands, TUI wizard, runtime), forge-plugins (channel plugins, markdown converter), forge-skills (skill parser, compiler, analyzer, trust)

Description

Eight high priority security vulnerabilities identified via cross-reference with OpenClaw 2026.2.19 / 2026.2.20. Exploitable under specific conditions or represent significant hardening gaps.

Re-verified 2026-03-12 against latest main (commit 7d2148b). Key changes from original analysis:

  • H-4 downgraded: Slack uses Socket Mode (WebSocket), not HTTP webhooks. No replay/dedup protection exists at all (previously thought HMAC was in place).
  • H-8 downgraded: cli_execute.go is secure, but custom_tool.go has unvalidated entrypoint paths — now tracked as a partial fix needing completion.

Reference: FORGE-SECURITY-UPDATE.md — Phase 2

Steps to reproduce

H-1. A2A Server Missing Rate Limiting

  • File: forge-cli/server/a2a_server.go
  • No rate limiting middleware exists. Handler chain (lines 124-130) includes only CORS and optional auth. Send unlimited requests to any endpoint.

H-2. A2A Server Missing Request Size Limits

  • File: forge-cli/server/a2a_server.go
  • handleJSONRPC() (line 196) uses json.NewDecoder(r.Body).Decode(&req) with no http.MaxBytesReader. HTTP server config (lines 132-136) does not set MaxHeaderBytes. Send a multi-GB payload to cause OOM.

H-3. Telegram Webhook Missing Signature Verification

  • File: forge-plugins/channels/telegram/telegram.go (lines 125–142)
  • makeWebhookHandler() reads raw body and calls NormalizeEvent() without any token verification. No Content-Type enforcement. Binds to 0.0.0.0 (line 106-109). POST any JSON to the webhook port to inject fake messages.

H-4. Slack Socket Mode — Missing Event Deduplication

  • File: forge-plugins/channels/slack/slack.go
  • Slack uses Socket Mode (WebSocket), not HTTP webhooks. The readLoop() (lines 258-391) processes all events immediately after acknowledging the envelope (lines 280-285) with no deduplication or replay protection. Duplicate events from Slack retries are processed multiple times.

H-5. Custom Tool npx ts-node Runtime Downloads

  • File: forge-core/tools/custom_tool.go (line 56)
  • return "npx", []string{"ts-node"} — downloads from npm if not installed. No --no-install flag, no sandboxing, no integrity validation.

H-6. Default Trust Policy Accepts Unsigned Local Skills

  • File: forge-skills/trust/types.go (lines 15–20)
  • DefaultTrustPolicy() returns MinTrustLevel: contract.TrustLocal, RequireChecksum: false, RequireSignature: false.

H-7. Skill Scanner Symlink Handling

  • File: forge-skills/local/scanner.go (lines 13–106)
  • Scan() uses fs.ReadDir() and fsys.Open(). No filepath.EvalSymlinks(). Directly concatenates name + "/SKILL.md" (line 33). Symlinks can point outside the skill directory.

H-8. Custom Tool Entrypoint Validation Gap

  • Files: forge-core/tools/custom_tool.go, forge-cli/tools/cli_execute.go
  • cli_execute.go is secure (explicit args, shell blocklist, argument validation). However, custom_tool.go (line 48) passes the entrypoint to the executor without validating the file path or checking arguments from JSON input.

H-10. Token/Secret Reuse Detection

  • Files: forge-core/types/config.go, forge-core/secrets/, forge-cli/runtime/runner.go
  • Startup flow (runner.go lines 117-142) calls ResolveAuth() and overlaySecrets() with no validation that secrets/tokens are unique across subsystems. chain_provider.go returns first match without dedup.

Expected behavior

  • H-1: Per-IP rate limiting with 429 Too Many Requests responses.
  • H-2: Request bodies limited to 2 MiB with 413 on excess.
  • H-3: Telegram webhook verifies X-Telegram-Bot-Api-Secret-Token, enforces JSON Content-Type, binds to localhost.
  • H-4: Duplicate Slack events detected by envelope_id and skipped.
  • H-5: npx --no-install ts-node prevents runtime downloads; clear error if not installed.
  • H-6: RequireChecksum: true by default; warnings on unsigned skills.
  • H-7: Symlinks resolved and validated to stay within trusted root.
  • H-8: Entrypoint validated as regular file within skill directory.
  • H-10: Startup rejects configs where distinct-purpose secrets are identical.

Actual behavior

  • H-1: Unlimited requests accepted — DoS risk.
  • H-2: Unlimited payload size — OOM risk.
  • H-3: Any HTTP client can inject fake Telegram messages.
  • H-4: Duplicate events processed multiple times causing duplicate bot responses.
  • H-5: npx downloads packages from npm registry at runtime.
  • H-6: Unsigned, unchecked skills accepted by default.
  • H-7: Symlinks followed without boundary checks.
  • H-8: Arbitrary entrypoint paths accepted without validation.
  • H-10: No duplicate token detection at startup.

Tasks

H-1. Rate Limiting

  • Add per-IP rate limiting middleware (golang.org/x/time/rate or token bucket)
  • Default: 60 req/min reads, 10 req/min writes
  • Return 429 with Retry-After header
  • Make limits configurable
  • Add tests

H-2. Request Size Limits

  • Wrap body reads with http.MaxBytesReader(w, r.Body, maxBytes)
  • Default: 2 MiB prompts, 10 MiB uploads
  • Return 413 on excess
  • Add tests for oversized payloads

H-3. Telegram Webhook Auth

  • Use setWebhook with secret_token (Bot API 6.1+)
  • Verify X-Telegram-Bot-Api-Secret-Token header
  • Enforce Content-Type: application/json
  • Bind to 127.0.0.1 by default
  • Add per-path rate limiting
  • Add tests for valid/invalid/missing token

H-4. Slack Event Deduplication

  • Add event-ID dedup cache (in-memory map with TTL)
  • Track envelope_id or event payload hash as dedup key
  • Skip processing for duplicates (acknowledge but don't act)
  • Add TTL-based eviction to prevent memory growth
  • Add tests for duplicate event rejection

H-5. npx Safety

  • Change to return "npx", []string{"--no-install", "ts-node"}
  • Or resolve ts-node binary path directly and fail if not found
  • Log clear error if not installed
  • Add test verifying --no-install present

H-6. Trust Policy Defaults

  • Change to RequireChecksum: true
  • Consider RequireSignature: true for production
  • Log warnings on unsigned skills
  • Add --trust-unsigned CLI flag for dev use

H-7. Symlink Handling

  • Resolve paths with filepath.EvalSymlinks()
  • Verify resolved path within trusted root
  • Reject paths resolving outside root
  • Log warnings on symlinks
  • Add tests for symlink escape

H-8. Entrypoint Validation

  • Validate entrypoint is a regular file (not symlink, device)
  • Ensure entrypoint resolves within expected skill directory
  • Validate JSON arguments for injection patterns
  • Add tests for malicious entrypoint paths

H-10. Token Reuse Detection

  • Collect all tokens/secrets at startup
  • Reject configs with duplicate secret values across subsystems
  • Log clear error identifying colliding tokens
  • Add test for duplicate detection

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingsecuritySecurity vulnerability fixes

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions