Security: fix prototype pollution in hasExplicitProviderAccountConfig (#34926)#35001
Security: fix prototype pollution in hasExplicitProviderAccountConfig (#34926)#35001alexyyyander wants to merge 2 commits intoopenclaw:mainfrom
Conversation
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 SummaryThis PR applies a focused, one-line security hardening fix in
Confidence Score: 5/5
Last reviewed commit: 084c8aa |
|
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
There was a problem hiding this comment.
💡 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".
| ? resolveGatewayCredentialsFromConfig({ | ||
| cfg, | ||
| env: process.env, | ||
| modeOverride: validatedOverride?.target, | ||
| }).token |
There was a problem hiding this comment.
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 👍 / 👎.
|
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. |
Summary
Fixes a prototype pollution vulnerability in the
hasExplicitProviderAccountConfigfunction insrc/security/audit-channel.ts.Fix
Replace the
inoperator withObject.hasOwn()to prevent prototype pollution attacks.Before:
After:
Security Impact
The
inoperator traverses the prototype chain, which means an attacker could supply__proto__orconstructoras theaccountIdparameter to bypass security checks.Object.hasOwn()only checks own properties, preventing this attack vector.Testing
Verified the fix works correctly:
Object.hasOwn(obj, "foo")returnstruefor existing own propertiesObject.hasOwn(obj, "__proto__")returnsfalse(prevents prototype pollution)Object.hasOwn(obj, "constructor")returnsfalse(prevents constructor pollution)This is the same fix pattern recommended by OWASP for preventing prototype pollution vulnerabilities.
Fixes #34926