GHSA-J4C5-89F5-F3PM: CDP Host Allowlist Separation Fix Review

Vulnerability

GHSA-J4C5-89F5-F3PM describes an SSRF bypass in OpenClaw where configuration parsing promoted backend CDP endpoint hostnames into the frontend browser navigation allowlist. The vulnerable behavior is visible in the earlier config path shown in commit e90c89cf8b1459f2aa1f3a665be67392b6c03fdf, where collectConfiguredCdpHostnames(cfg) extracted hostnames from cdpUrl values and passed them into resolveBrowserSsrFPolicy. That merged infrastructure-facing CDP destinations into the shared SSRF policy used for browser navigation.

This is a trust-boundary failure: a host needed for backend CDP control is not equivalent to a host safe for arbitrary browser navigation. Once a private or internal CDP host was inserted into allowedHostnames, navigation checks could treat that host as explicitly permitted, weakening protections intended to block access to private, loopback, or special-use addresses.

function collectConfiguredCdpHostnames(cfg: BrowserConfig | undefined): string[] {
  const hostnames = new Set<string>();
  addHostnameFromUrl(cfg?.cdpUrl);
  for (const profile of Object.values(cfg?.profiles ?? {})) {
    addHostnameFromUrl(profile?.cdpUrl);
  }
  return Array.from(hostnames);
}

ssrfPolicy: resolveBrowserSsrFPolicy(cfg, collectConfiguredCdpHostnames(cfg))

The advisory summary is consistent with the regression evidence in the code and tests: profile CDP configuration widened the browser SSRF allowlist, enabling requests into otherwise restricted internal networks.

Patch

The remediation in commit 1fd049e3074cac72f6734a7fe88468c84f5f8bd7 removes profile-derived CDP hostnames from global browser SSRF resolution and introduces a dedicated CDP reachability policy path. In config.ts, resolveBrowserSsrFPolicy no longer accepts extra hostnames, and the collectConfiguredCdpHostnames flow is removed from browser config resolution. In cdp-reachability-policy.ts, the new helper withCdpHostnameAllowed selectively augments only the CDP policy, and only when a profile has a cdpHost, an SSRF policy exists, and the policy does not already allow private networks globally.

function withCdpHostnameAllowed(
  profile: ResolvedBrowserProfile,
  ssrfPolicy?: SsrFPolicy,
): SsrFPolicy | undefined {
  if (!ssrfPolicy || !profile.cdpHost) {
    return ssrfPolicy;
  }
  if (isPrivateNetworkAllowedByPolicy(ssrfPolicy)) {
    return ssrfPolicy;
  }
  return {
    ...ssrfPolicy,
    allowedHostnames: Array.from(
      new Set([...(ssrfPolicy.allowedHostnames ?? []), profile.cdpHost]),
    ),
  };
}

The tests added in cdp-reachability-policy.test.ts are the strongest evidence that the intended security property is restored. One test explicitly verifies that the selected remote profile CDP host is allowed for CDP reachability while the browser navigation policy remains unchanged, and that navigation to http://172.29.128.1/ is still rejected. The config tests were also updated to assert that configured profile cdpUrl values stay out of the shared browser SSRF policy. The changelog entry states the same design intent: allow the selected remote CDP profile host for CDP health/control checks without widening browser navigation SSRF policy.

Review

Pros

• The patch addresses the core architectural mistake: it separates backend CDP reachability from frontend navigation authorization instead of merely filtering a few cases.
• Security intent is encoded in tests. The new test asserts both positive behavior for CDP access and negative behavior for browser navigation against the same private IP.
• The implementation is narrow and state-preserving: it augments only the effective CDP policy for the selected profile rather than mutating shared configuration.
• Deduplication via Set avoids repeated hostname entries, and the loopback case is explicitly kept outside browser SSRF policy.
• The guard isPrivateNetworkAllowedByPolicy(ssrfPolicy) avoids unnecessary special-casing when the deployment has already opted into broader private-network access.

Cons

• The source set includes commit e90c89cf8b1459f2aa1f3a665be67392b6c03fdf, which reintroduces the vulnerable merge behavior in the provided snippets. Reviewers and release engineers need to ensure that this commit is not the effective shipped state after the fix commit.
• There is naming inconsistency in the test snippets between allowedHostnames and hostnameAllowlist. That may be benign if both are supported or one is test scaffolding, but it is worth verifying to avoid policy drift or dead configuration paths.
• The patch is scoped to hostname separation. It does not, from the provided snippets alone, demonstrate broader normalization hardening around alternate IP encodings, DNS rebinding, or scheme restrictions; those concerns may be handled elsewhere, but they are not evidenced here.

Verdict

Root-cause.

The final remediation shown in commit 1fd049e3074cac72f6734a7fe88468c84f5f8bd7 fixes the actual design flaw by decoupling CDP control-plane exceptions from browser navigation SSRF policy. That is the correct security boundary. The earlier referenced commit e90c89cf8b1459f2aa1f3a665be67392b6c03fdf reflects the vulnerable approach and should be treated as historical context, not the fix. Assuming the fixed commit is what shipped, the patch is technically sound and well-tested against the reported SSRF bypass scenario. For additional context, see the advisory at GitHub Security Advisory and the external report at cvereports.com.