cloudflare / workers-sdk

⛅️ Home to Wrangler, the CLI for Cloudflare Workers®
https://developers.cloudflare.com/workers/
Apache License 2.0
2.72k stars 716 forks source link

🐛 BUG: Prototype pollution leads to crash in wrangler #5111

Open Cherry opened 8 months ago

Cherry commented 8 months ago

Which Cloudflare product(s) does this pertain to?

Wrangler core

What version(s) of the tool(s) are you using?

3.29.0

What version of Node are you using?

20.11.1

What operating system and version are you using?

Windows

Describe the Bug

Observed behavior

By using the __proto__ key in certain conditions, you can crash the wrangler process. A smarter actor than myself may be able to exploit this for deeper prototype pollution.

Steps to reproduce

[vars] proto = {evil = true}

Or:
```toml
compatibility_date = "2022-01-12"
name = "test"
main = "./src/index.ts"

[[kv_namespaces]]
id = "test-namespace"
binding = "__proto__"

(etc);.

src/index.ts:

export interface Env {}
export default {
    async fetch(request: Request, env: Env, ctx: ExecutionContext): Promise<Response> {
        return Response.json(env)
    },
};

Or full stack trace (observed when editing the wrangler.toml of a previously functional worker:

TypeError: bindingsGroupedByName[bindingName].push is not a function
    at validateBindingsHaveUniqueNames (E:\GitHub\white-water-2a7c\node_modules\wrangler\wrangler-dist\cli.js:123686:42)
    at normalizeAndValidateConfig (E:\GitHub\white-water-2a7c\node_modules\wrangler\wrangler-dist\cli.js:121861:3)
    at readConfig (E:\GitHub\white-water-2a7c\node_modules\wrangler\wrangler-dist\cli.js:124003:35)
    at FSWatcher.<anonymous> (E:\GitHub\white-water-2a7c\node_modules\wrangler\wrangler-dist\cli.js:167867:18)
    at FSWatcher.emit (node:events:518:28)
    at FSWatcher.emit (node:domain:488:12)
    at FSWatcher.emitWithAll (E:\GitHub\white-water-2a7c\node_modules\chokidar\index.js:540:8)
    at FSWatcher._emit (E:\GitHub\white-water-2a7c\node_modules\chokidar\index.js:632:8)
    at listener (E:\GitHub\white-water-2a7c\node_modules\chokidar\lib\nodefs-handler.js:370:20)
penalosa commented 8 months ago

There are a couple different approaches we could take here to make this experience better, from ignoring keys named __proto__ to erroring if an invalid key is present.

What behaviour would you expect from Wrangler in this case?

Cherry commented 8 months ago

I think ignoring keys like __proto__ is the most common mitigation to this, along with Object.create(null) in places over {}.