make-fetch-happen icon indicating copy to clipboard operation
make-fetch-happen copied to clipboard

[BUG] agent.js caches proxy w/o respect for credentials

Open japhy- opened this issue 5 years ago • 0 comments

What

getAgent() in agent.js serializes the proxy based only on the proxy protocol, hostname, and port, disregarding the credentials (username and password). This causes a bug that prevents the library from using the same proxy multiple times with different credentials.

How

Current Behavior

agent.js, lines 30-42:

  const key = [
    `https:${isHttps}`,
    pxuri
      ? `proxy:${pxuri.protocol}//${pxuri.host}:${pxuri.port}`
      : '>no-proxy<',
    `local-address:${opts.localAddress || '>no-local-address<'}`,
    `strict-ssl:${isHttps ? !!opts.strictSSL : '>no-strict-ssl<'}`,
    `ca:${(isHttps && opts.ca) || '>no-ca<'}`,
    `cert:${(isHttps && opts.cert) || '>no-cert<'}`,
    `key:${(isHttps && opts.key) || '>no-key<'}`,
    `timeout:${agentTimeout}`,
    `maxSockets:${agentMaxSockets}`
  ].join(':')

Steps to Reproduce

fetch(url, {proxy: "https://user1:[email protected]:1234"}); // uses https://user1:[email protected]:1234
fetch(url, {proxy: "https://user2:[email protected]:1234"}); // ALSO uses https://user1:[email protected]:1234

Expected Behavior

fetch(url, {proxy: "https://user1:[email protected]:1234"}); // uses https://user1:[email protected]:1234
fetch(url, {proxy: "https://user2:[email protected]:1234"}); // uses https://user2:[email protected]:1234

Recommended Patch

  const key = [
    `https:${isHttps}`,
    pxuri
      ? `proxy:${pxuri.href}` // or 
      : '>no-proxy<',
    `local-address:${opts.localAddress || '>no-local-address<'}`,
    `strict-ssl:${isHttps ? !!opts.strictSSL : '>no-strict-ssl<'}`,
    `ca:${(isHttps && opts.ca) || '>no-ca<'}`,
    `cert:${(isHttps && opts.cert) || '>no-cert<'}`,
    `key:${(isHttps && opts.key) || '>no-key<'}`,
    `timeout:${agentTimeout}`,
    `maxSockets:${agentMaxSockets}`
  ].join(':')

If it is a security risk to include the username and password in the agent cache key, then a hashed version of pxuri.href can be used instead.

japhy- avatar Aug 10 '20 18:08 japhy-