-
Notifications
You must be signed in to change notification settings - Fork 336
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Defense against prototype pollution and supply chain attacks #412
Comments
Thanks a lot for all the effort @kriskowal. Happy to support this work where possible. For the |
I’m delighted to share, |
I’ve taken the liberty to propose this change with staltz/xstream#315 |
|
This is just a devDependency of xstream. Does this really matter for as as users of xstream? |
I suspect that it will be hard to land the version bump to |
Ben Lesh released I’ve posted a change for MostJS that brings it up to speed with |
And, attempting to upgrade |
Summary of what’s happened so far:
The pending work, in steps, is:
|
@kriskowal could you double check |
@webmaster128 I’ll be sure to. The next, and I presume last, step is for me to propose a change that adds SES to the test scaffold to confirm that CosmJS is now SES-compatible and to prove against regressions going forward. |
@kriskowal what do you mean by "test scaffold"? Would it be possible for us to pick up your work from here or does this require some work in the SES-shim repo? |
@webmaster128 There’s an opportunity in the jasmine-test-runner scripts in each package to import a module early in initialization that in turn imports |
Investigating the error from the https://github.com/cosmos/cosmjs/blob/4b7a060/packages/cli/src/async.ts#L9 Looking into |
The missing name that the error complains about has those 3 different values when running without SES:
This does not make much sense to me. Why is I guess this requires a larger debugging session. |
At @Agoric, we’ve begun investigating how compatible CosmJS is with SES such that projects using CosmJS can use tools like SES and LavaMoat to mitigate supply chain and prototype pollution attacks.
To evaluate whether a module and its transitive dependencies can be initialized under SES, I’ve locally run the following script in each of the CosmJS library packages. These are the packages that all have a single entry-module named
./build/index.js
.This provides some basic assurance that these libraries could be used in a project using SES. Likely incompatibilities usually arise when a library depends on a shim or polyfill, and these can usually be fixed by using the corresponding “ponyfill”, like
colors/safe
instead ofcolors
. (Though I would note, CosmJS depends much more heavily on the equivalentansi-colors
module that does not perform anyprototype attacksmonkey-patching.)The following depend on a broken ponyfill,
observable-symbol/ponyfill
, which we haved worked with Ben Lesh to fix. This fix comes in a new major version, so an explicit upgrade should get the remaining libraries working.benlesh/symbol-observable#48
Of the two applications,
faucet
andcli
, the faucet appears to work with the SES lockdown snippet at the head of itsbin
.The CLI runs into this mysterious error, far far from any likely cause:
This is a great place to start. From this position, we would ideally add SES lockdown to the testing aparatus for each of these packages, so every pull request against them would be verified to remain compatible.
To this end, there are a number of ways that Jasmine and Karma are themselves not yet ready to run under environmental lockdown, largely because they use the infamous
colors
package, which in addition to monkey-patchingString.prototype
upon initialization, provides a “themes” feature that enables users to indirectly punchString.prototype
at runtime.Karma should work with this proposed and accepted fix to use
colors/safe
instead. It would be nice if it usedansi-colors
just to reduce duplication among your transitive dependencies, but harmless otherwise.karma-runner/karma#3548
Jasmine itself is largely compatible, because the only primordial it modifies is
globalThis
, and SES lockdown leaves this as an exercise to the user.Jasmine Test Reporter makes uses of
colors
themes.To overcome this obstacle, we would need either an alternate test reporter plugin or a major version bump and an architecture more closely aligned with dependency injection. I’ve proposed the changes. The author finds them amenable. Work would need to be planned.
bcaudan/jasmine-spec-reporter#528
The nature of SES compatibility exploration is that it is easier to keep than to obtain. There may be further obstacles behind these first exceptions I’ve encountered, but overall, it seems CosmJS is very nearly already compatible with SES and could encourage users to use it (and LavaMoat) to protect their applications from these kinds of attack.
The text was updated successfully, but these errors were encountered: