Skip to content
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

SwingSet: Validate bundles are exclusively executable files #4811

Open
kriskowal opened this issue Mar 10, 2022 · 2 comments
Open

SwingSet: Validate bundles are exclusively executable files #4811

kriskowal opened this issue Mar 10, 2022 · 2 comments
Assignees
Labels
enhancement New feature or request SwingSet package: SwingSet

Comments

@kriskowal
Copy link
Member

kriskowal commented Mar 10, 2022

Following #4719 (validate bundles)

What is the Problem Being Solved?

To disincentivize the use of contract bundles to grief users of hypothetical contract explorers that display non-code files with abusive images or worse, SwingSet should in addition to verifying the integrity of a bundle, ensure that every file in the bundle is a valid executable code.

Description of the Design

Beside using @endo/check-bundle ascheckBundle in SwingSet, SwingSet should go on to read the content of the Zip file, walk the compartment map, and module-for-module, verify that every module parses according to its prescribed language. Endo Zip archives contain a compartment-map.json that describes every other file in the archive with a language attribute which is one of pre-mjs-json, pre-cjs-json, or json, corresponding to ESM, CommonJS, and JSON source files. The precompiled static module record formats are a JSON blob that contains a functor string, for both MJS and CJS languages. The validator would need to unpack the JSON and also attempt to construct Function from the subcontained functor, and also verify the JSON schema in general, disallowing unknown properties.

Security Considerations

The validator must not execute any contract code.

Contract code must not be able to induce arbitrary execution with crafted source.

The JavaScript parser involved must have a Function constructor that disallows an invalid function body. Some function constructors lazily assume that the function body is valid and inject the argument names and body into a template, which can bypass the implied validity check. This can be overcome by avoiding the use of the argument variables. So, new Function(functor) instead of new Function('require', 'exports', functor) [sic] should be sufficient, since we do not intend to use the resulting function. We should not need Babel for this, so we avoid some marginal supply chain risk.

Test Plan

This must be tested with contrived attack bundles. For example, a bundle that lists an MJS module that does not parse as JSON, or does parse as JSON but contains an invalid JavaScript functor, or contains an extraneous property, &c.

@kriskowal kriskowal added the enhancement New feature or request label Mar 10, 2022
@kriskowal kriskowal self-assigned this Mar 10, 2022
@dckc
Copy link
Member

dckc commented Mar 10, 2022

Please let's consider the cost in testing / development time of this change while we're at it. Please measure the time that agoric start takes to get to Wallet Deployed! before and after before merging this change. (cf #3802)

@kriskowal
Copy link
Member Author

Deferred to MN >1, tentatively MN2, per consensus with @warner at standup this morning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request SwingSet package: SwingSet
Projects
None yet
Development

No branches or pull requests

3 participants