ethers-io / ethers.js

Complete Ethereum library and wallet implementation in JavaScript.
https://ethers.org/
MIT License
7.96k stars 1.85k forks source link

Fix DoS vulnerability caused by `ws` dependency on v5 #4791

Open cgero-eth opened 3 months ago

cgero-eth commented 3 months ago

Ethers Version

5.7.2

Search Terms

ws, vulnerability, DoS, v5

Describe the Problem

Ethers.js v5.7.2 depends on a vulnerable version of the ws package. The vulnerability allows DoS attack. The ws package must be updated to version >= 8.17.1 to fix the vulnerability.

The vulnerability was reported by Ryan LaPointe in https://github.com/websockets/ws/issues/2230.

Code Snippet

From Dependabot:

const http = require('http');
const WebSocket = require('ws');

const wss = new WebSocket.Server({ port: 0 }, function () {
  const chars = "!#$%&'*+-.0123456789abcdefghijklmnopqrstuvwxyz^_`|~".split('');
  const headers = {};
  let count = 0;

  for (let i = 0; i < chars.length; i++) {
    if (count === 2000) break;

    for (let j = 0; j < chars.length; j++) {
      const key = chars[i] + chars[j];
      headers[key] = 'x';

      if (++count === 2000) break;
    }
  }

  headers.Connection = 'Upgrade';
  headers.Upgrade = 'websocket';
  headers['Sec-WebSocket-Key'] = 'dGhlIHNhbXBsZSBub25jZQ==';
  headers['Sec-WebSocket-Version'] = '13';

  const request = http.request({
    headers: headers,
    host: '127.0.0.1',
    port: wss.address().port
  });

  request.end();
});

Contract ABI

N/A

Errors

N/A

Environment

Ethereum (mainnet/ropsten/rinkeby/goerli), Altcoin - Please specify (e.g. Polygon), node.js (v12 or newer), Browser (Chrome, Safari, etc)

Environment (Other)

No response

ricmoo commented 3 months ago

Thanks for the snippet.

This was addressed when the exploit was announced in v6, but it certainly makes sense to port back into v5 as it is still widely used.

netzulo commented 2 months ago

lol, just a merge need it on v5 ? please, resolve this, easy to close :D

ricmoo commented 2 months ago

It’s not really “easy to close”… The v5 branch is 2.5 years old; I’ve spent the last week carefully updating the tests (migrating from Goerli to Sepolia, redeploying contracts, etc), deployment scripts and other dev dependencies though and it is almost ready with the updated ws package. :)

It still has 650k downloads per week though, so important not to bork and make sure the change is well tested. :)

airdropross commented 2 months ago

Hey @ricmoo any status update here? We are also trying to patch this vulnerability in our codebase and would love to continue using your incredible ethers library!

ricmoo commented 1 month ago

@airdropross I'm still working on this. The problem is that many of the devtools that ethers v5 depended on have drifted out of support.

One of the biggest issues has been Karma has become deprecated, which is responsible to the CI ensuring correct behaviour in browsers. In v6 I wrote my own tool to replace it, but since v5 is over 2 years old, it's been harder to figure out the best path forward; porting the v6 test framework back to v5 or try a more recent (but still deprecated Karma) package.

I believe I have found a viable path forward though, at least for the near future for keeping v5 tested before publishing.

I'd also love to help projects out, if they need it, to migrate to v6. Is there a reason for holding off migration to v6? Dependencies? I am looking for information on those too, to help dependencies migrate. :)

silverpill commented 1 month ago

I'd also love to help projects out, if they need it, to migrate to v6. Is there a reason for holding off migration to v6? Dependencies? I am looking for information on those too, to help dependencies migrate. :)

@ricmoo For me one of the reasons is bundle size. Ethers is already the heaviest dependency in my app and migrating from 5.6.9 to 6.13.2 adds another 42 kBytes (I only use Web3Provider; the bundler is Rollup which should do tree-shaking)