Open kriskowal opened 2 years ago
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)
Deferred to MN >1, tentatively MN2, per consensus with @warner at standup this morning.
Following https://github.com/Agoric/agoric-sdk/issues/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 acompartment-map.json
that describes every other file in the archive with alanguage
attribute which is one ofpre-mjs-json
,pre-cjs-json
, orjson
, 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 constructFunction
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 ofnew 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.