Skip to content

Security: fix prototype pollution in hasExplicitProviderAccountConfig (#34926)#35001

Closed
alexyyyander wants to merge 2 commits intoopenclaw:mainfrom
alexyyyander:fix/34926-prototype-pollution
Closed

Security: fix prototype pollution in hasExplicitProviderAccountConfig (#34926)#35001
alexyyyander wants to merge 2 commits intoopenclaw:mainfrom
alexyyyander:fix/34926-prototype-pollution

Conversation

@alexyyyander
Copy link
Copy Markdown
Contributor

Summary

Fixes a prototype pollution vulnerability in the hasExplicitProviderAccountConfig function in src/security/audit-channel.ts.

Fix

Replace the in operator with Object.hasOwn() to prevent prototype pollution attacks.

Before:

return accountId in accounts;

After:

return Object.hasOwn(accounts, accountId);

Security Impact

The in operator traverses the prototype chain, which means an attacker could supply __proto__ or constructor as the accountId parameter to bypass security checks. Object.hasOwn() only checks own properties, preventing this attack vector.

Testing

Verified the fix works correctly:

  • Object.hasOwn(obj, "foo") returns true for existing own properties
  • Object.hasOwn(obj, "__proto__") returns false (prevents prototype pollution)
  • Object.hasOwn(obj, "constructor") returns false (prevents constructor pollution)

This is the same fix pattern recommended by OWASP for preventing prototype pollution vulnerabilities.

Fixes #34926

Replace 'in' operator with Object.hasOwn() to prevent prototype
pollution attacks. The 'in' operator traverses the prototype chain,
allowing attackers to bypass security checks using '__proto__' or
'constructor' as accountId.

Fixes openclaw#34926
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 4, 2026

Greptile Summary

This PR applies a focused, one-line security hardening fix in src/security/audit-channel.ts, replacing the in operator with Object.hasOwn() in hasExplicitProviderAccountConfig to prevent prototype-chain traversal when checking whether an accountId has explicit configuration.

  • The fix is correct: Object.hasOwn(accounts, accountId) restricts the lookup to own properties, ensuring keys like __proto__, constructor, or toString (which are always present via the prototype chain) are not falsely reported as explicit account configs.
  • No behavioral regression for normal account IDs — only prototype-inherited keys would have been affected by the old code, and those are not valid account IDs in any intended use case.
  • Object.hasOwn is available in Node.js 16.9.0+ / ES2022 and is the OWASP-recommended replacement pattern; no compatibility concerns for a modern TypeScript project.
  • All other object iterations in the file (Object.entries, Object.keys, Object.values) are already own-property-safe, so no further changes are required.

Confidence Score: 5/5

  • This PR is safe to merge — the change is minimal, correct, and carries no risk of regression.
  • The diff is a single-line substitution of a well-understood pattern. Object.hasOwn is the canonical replacement for in when prototype-chain traversal must be avoided. The rest of the file already uses own-property-safe APIs, and the fix has no side effects on normal (non-prototype-key) account IDs.
  • No files require special attention.

Last reviewed commit: 084c8aa

@benediktjohannes
Copy link
Copy Markdown
Contributor

Hello @alexyyyander,

thank you very much for your contribution. I really appreciate your work. Since we already have another pull request that was opened earlier (#34929) addressing this issue, I recommend commenting on the other pull request if you would like to suggest any changes. Thank you very much nevertheless.

cc @shakkernerd, could you please close this so that we don’t run into confusion with multiple PRs? Thanks!

When gateway.auth.mode is set to 'token', agent tool calls (e.g.,
sessions_spawn, cron, sessions_list) fail with 'device-required' error
because resolveGatewayOptions doesn't fallback to reading the token from
OPENCLAW_GATEWAY_TOKEN environment variable.

This fix adds a fallback to resolveGatewayCredentialsFromConfig when no
explicit token is provided via opts, matching the behavior of the CLI path
which properly reads from environment variables.

Fixes openclaw#35039
@openclaw-barnacle openclaw-barnacle bot added the agents Agent runtime and tooling label Mar 4, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 854e1d3d14

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +129 to +133
? resolveGatewayCredentialsFromConfig({
cfg,
env: process.env,
modeOverride: validatedOverride?.target,
}).token
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Do not promote implicit config creds to explicit gatewayToken

This fallback now resolves credentials from config/env before calling callGateway, then passes the result as opts.token (explicit auth). In the OPENCLAW_GATEWAY_URL override path, callGateway is supposed to restrict implicit credential reuse by resolving auth with urlOverrideSource: "env" (env-only) and enforcing explicit-auth semantics, but this change bypasses that by turning config-derived tokens into explicit tokens up front. A process with OPENCLAW_GATEWAY_URL set and a configured gateway token will now send that token to the override URL, which reintroduces credential exfiltration risk for env URL overrides.

Useful? React with 👍 / 👎.

@dvrshil
Copy link
Copy Markdown
Contributor

dvrshil commented Mar 5, 2026

Thanks for the work, @alexyyyander. This issue has been addressed by maintainer-selected PR #34982 (tracking issue #34926).\n\nTo keep the queue clean, I’m closing this as a duplicate.

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

Labels

agents Agent runtime and tooling size: XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Security]: Use of in operator in hasExplicitProviderAccountConfig can lead to prototype pollution bypass

3 participants