Ethers.js: Preventing property and prototype manipulation

[rstormsf]

https://github.com/ethereum/web3.js/issues/2646#issuecomment-483588301
from nivida

We do here security sensitive stuff and ethers.js should probably also implement such strict restrictions.

[ricmoo]

Sorry, I don’t quite understand the ask.

I don’t currently use Proxy anywhere internally, as their support is fairly limited. They are also incredible slow (last I experimented with them), although that itself would not really be a reason to not use them, as they can be incredibly useful, especially for meta classes.

There should not be anything preventing you from using Proxy.revokable though, with ethers. Can you provide some sample code that fails?

[rstormsf]

I think @nivida meant that ethers.js should not be able to let some 3rd party libs (like vue-devtool) to reinitialize the library. I don't think it's related to Proxies at all.

@ricmoo

[nivida]

The angular and rxjs core team told me that the performance itself isn’t that bad and will be improved over the time.

[rstormsf]

I guess the main question is how ethers.js can have "full SES" implemented, according to your standards @nivida

[nivida]

This will be really hard because of the architecture. But I think @ricmoo is improving the architecture with v5 and we could probably work at it after. Would be great to combine the forces for some modules.

[ricmoo]

Is the plan just to have a copy of the library with all properties and prototypes frozen? That seems like the easiest thing, but not sure if that is sufficient. V3 was like that, but it does not play well with TypeScript, so v4 had to abandon that. Maybe include a frozen ethers at the top level?

[nivida]

Yes, the goal should be to have the objects frozen. Just adding an Object.freeze() on the top level doesn't work. The Proxy object provides some additional features for having a custom and still strict frozen JS object.

Docs Object.freeze:

Note that values that are objects can still be modified, unless they are also frozen. As an object, an array can be frozen; after doing so, its elements cannot be altered and no elements can be added to or removed from the array.

[nivida]

This Mozilla Hacks blog post is a good resource: https://hacks.mozilla.org/2015/07/es6-in-depth-proxies-and-reflect/

[ricmoo]

Right, it would need to freeze recursively, and include all prototypes, specifically.

The issue with TypeScript is that once a prototype is frozen, it cannot be sub-classed, which is why I was thinking of doing it as part of the ethers import. Maybe expose a secure? So, if you wanted, you could use import { secure as ethers } from “ethers” for example.

[nivida]

Yes, or you implement it with a Proxy. I think this should work right?
(We could create a testing repository where we do some examples of SES together)

[ricmoo]

Don’t forget that Proxy is a new feature and not available in many environments, so there would be a non-trivial number of developers that would lose access if Proxy is used. Just using it in the exports would prevent importing.

I already have a recursive freeze I use in some of my other projects. I’ll add it to the v5 branch today to start experimenting. :)

I think it is still important for most functionality to work in ES3 environments.

[rstormsf]

@ricmoo I don't think supporting ES3 is good idea. If someone wants to develop something for ethereum, they should probably use modern runtime env.

[ricmoo]

That’s seems like an unfair ask. It is not hard to continue using ES3-esque environments or mostly basic ES5 features. It just means we cannot use things like Proxy (which are ES6 anyways).

Lots of existing applications and tools are build on a wide variety of platforms, like React Native, Expose, Otto, Embedded JavaScript environments, PhantomJS, Chromium, electron and not to mention non-modern browsers and node in general. Many of these environments are behind the curve to catch up with the latest node, V8 engine or WebKit. And many projects do not have time to catch up on migrating to new versions of these frameworks or the framework may have been discontinued, so they would need to migrate to a new one.

Many people forget just how diverse the JavaScript ecosystem is out there, because they are used to their environment, tools and platform. It is important though, that we continue to support as many people as is reasonable. The next major version of ethers may support only ES5 and above, but that will be communicated long in advance, just as I have already communicated whenever relevant that this version of ethers will continue to work in all environments it currently works in.

Live and let live. :)

[nivida]

@ricmoo Most runtimes do support proxies for a longer time: https://kangax.github.io/compat-table/es6/

[ricmoo]

Well, I personally use multiple environments which don't support Proxies, so that is also a problem.

For one thing the entire test framework would collapse, but I plan to re-write a lot of that to use headless Chromium in the future, since phantomjs is no longer maintained. But there are also embedded environments and minimal JavaScript implements which don't support them, which I occasionally use. Not sure if Ductap or Otto has support for them yet, or if they plan to add them.

Regardless, it is very easy to solve this without using Proxies (v3 had it), so why add a restrictive dependency that is not necessary?

[ricmoo]

There are JavaScript environments coming out of the woodwork that are attempting to fix this, but for now I think this is out of the scope of ethers. I'm going to close this, but if you'd like to continue discussions please feel free to re-open or message me directly on Gitter or Discord.

Thanks! :)