Version Details and Security Vulnerabilities

Impact

This is a hole in the confinement of guest applications under SES that may manifest as either the ability to exfiltrate information or execute arbitrary code depending on the configuration and implementation of the surrounding host.

Guest program running inside a Compartment with as few as no endowments can gain access to the surrounding host’s dynamic import by using dynamic import after the spread operator, like {...import(arbitraryModuleSpecifier)}.

On the web or in web extensions, a Content-Security-Policy following ordinary best practices likely mitigates both the risk of exfiltration and execution of arbitrary code, at least limiting the modules that the attacker can import to those that are already part of the application. However, without a Content-Security-Policy, dynamic import can be used to issue HTTP requests for either communication through the URL or for the execution of code reachable from that origin.

Within an XS worker, an attacker can use the host’s module system to the extent that the host has been configured. This typically only allows access to module code on the host’s file system and is of limited use to an attacker.

Within Node.js, the attacker gains access to Node.js’s module system. Importing the powerful builtins is not useful except insofar as there are side-effects and tempered because dynamic import returns a promise. Spreading a promise into an object renders the promises useless. However, Node.js allows importing data URLs, so this is a clear path to arbitrary execution.

Patches

All affected 0.* version trains have the following patch. Running npm update will obtain the patch on all affected projects using ^0.* style dependency constraints in their package.json.

From 33469e88bfb2bf34a161c265f10f808ce354a700 Mon Sep 17 00:00:00 2001
From: Kris Kowal <kris@agoric.com>
Date: Thu, 27 Jul 2023 13:25:13 -0700
Subject: [PATCH] fix(fix): Censor spread import

---
 packages/ses/src/transforms.js       |  2 +-
 packages/ses/test/test-transforms.js | 22 +++++++++++++++++++++-
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/packages/ses/src/transforms.js b/packages/ses/src/transforms.js
index a0fc8d0ef..64a46cb53 100644
--- a/packages/ses/src/transforms.js
+++ b/packages/ses/src/transforms.js
@@ -106,7 +106,7 @@ export const evadeHtmlCommentTest = src => {
 // /////////////////////////////////////////////////////////////////////////////
 
 const importPattern = new FERAL_REG_EXP(
-  '(^|[^.])\\bimport(\\s*(?:\\(|/[/*]))',
+  '(^|[^.]|\\.\\.\\.)\\bimport(\\s*(?:\\(|/[/*]))',
   'g',
 );
 
diff --git a/packages/ses/test/test-transforms.js b/packages/ses/test/test-transforms.js
index cef0c02c1..8f6818b83 100644
--- a/packages/ses/test/test-transforms.js
+++ b/packages/ses/test/test-transforms.js
@@ -6,7 +6,7 @@ import {
 } from '../src/transforms.js';
 
 test('no-import-expression regexp', t => {
-  t.plan(9);
+  t.plan(13);
 
   // Note: we cannot define these as regular functions (and then stringify)
   // because the 'esm' module loader that we use for running the tests (i.e.
@@ -20,6 +20,7 @@ test('no-import-expression regexp', t => {
   const safe = 'const a = 1';
   const safe2 = "const a = notimport('evil')";
   const safe3 = "const a = importnot('evil')";
+  const safe4 = "const a = compartment.import('name')";
 
   const obvious = "const a = import('evil')";
   const whitespace = "const a = import ('evil')";
@@ -27,10 +28,14 @@ test('no-import-expression regexp', t => {
   const doubleSlashComment = "const a = import // hah\n('evil')";
   const newline = "const a = import\n('evil')";
   const multiline = "\nimport('a')\nimport('b')";
+  const spread = "{...import('exfil')}";
+  const spread2 = "{\n...\nimport\n('exfil')}";
+  const spread3 = "{\n...\nimport/**/\n('exfil')}";
 
   t.is(rejectImportExpressions(safe), safe, 'safe');
   t.is(rejectImportExpressions(safe2), safe2, 'safe2');
   t.is(rejectImportExpressions(safe3), safe3, 'safe3');
+  t.is(rejectImportExpressions(safe4), safe4, 'safe4');
   t.throws(
     () => rejectImportExpressions(obvious),
     { instanceOf: SyntaxError },
@@ -62,6 +67,21 @@ test('no-import-expression regexp', t => {
     'possible import expression rejected around line 2',
     'multiline',
   );
+  t.throws(
+    () => rejectImportExpressions(spread),
+    { instanceOf: SyntaxError },
+    'spread',
+  );
+  t.throws(
+    () => rejectImportExpressions(spread2),
+    { instanceOf: SyntaxError },
+    'spread2',
+  );
+  t.throws(
+    () => rejectImportExpressions(spread3),
+    { instanceOf: SyntaxError },
+    'spread3',
+  );
 });
 
 test('no-html-comment-expression regexp', t => {
-- 
2.40.1

Workarounds

On the web, providing a suitably constrained Content-Security-Policy mitigates most of the threat.

With XS, building a binary that lacks the ability to load modules at runtime mitigates the entirety of the threat. That will look like an implementation of fxFindModule in a file like xsPlatform.c that calls fxRejectModuleFile.

We highly advise applying the above patch for Node.js as there is no known work-around and Node.js’s module specifiers are exceedingly powerful, including support for data:text/javascript, style module specifier URLs.

References

No references at this time.

Impact

Web pages and web extensions using ses and the Compartment API to evaluate third-party code in an isolated execution environment that have also elsewhere used const, let, and class bindings in the top-level scope of a <script> tag will have inadvertently revealed these bindings in the lexical scope of third-party code.

Patches

This compromise is addressed in ses version 1.12.0. The mechanism for confining third-party code involves a with block and a semi-opaque scope Proxy. The proxy previously revealed any named property to the surrounding lexical scope if it were absent on globalThis, so that the third-party code would receive an informative ReferenceError, relying on the invalid assumption that only properties of globalThis are in the top-level lexical scope. The solution makes the scope proxy fully opaque. Consequently, accessing an unbound free lexical name will produce undefined instead of throwing ReferenceError. Assigning to an unbound free lexical name will continue to throw a ReferenceError.

Workarounds

This problem can be mitigated either by avoiding top-level let, const, or class bindings in <script> tags, which is an existing industry best-practice, or change these to var bindings to be reflected on globalThis, or upgrade ses to version 1.12.0 or greater.

Some bundlers by default transform top-level let, const, and class bindings to var.

Disclosure

This vulnerability was disclosed by @mingijunggrape in the course of their studies at UNIST (Ulsan National Institute of Science and Technology) as a member of the Web Security Lab (https://websec-lab.github.io/).

Impact

What kind of vulnerability is it? Who is impacted?

In Hardened JavaScript, programs can harden objects to safely share objects with co-tenant programs without risk of these other programs tampering with their API surface. Hardening does not guarantee that objects are pure or immutable, so a hardened Map, for example is superficially tamper-proof, but any party holding a reference to the object can both read and write its contents. Based on this precedent, and because TypedArray instances cannot be frozen with Object.isFrozen, harden does not freeze TypedArrays and instead makes them non-extensible and makes all non-indexed properties non-writable and non-configurable. This is consistent with the treatment of Map because the indexed properties represent mutable content and non-indexed properties represent the API.

Due to a defect in harden, properties that have names that parse as numbers but are not the same as the canonical representation of those numbers, as in "+0" and "" which are both equivalent to their canonical number "0", remain writable after hardening.

Any program treating one of these properties as part of its API and relying on harden to prevent modifications would be vulnerable to an API pollution attack, affecting only instances shared by mutually suspicious parties.

Unlike a Map, a hardened TypedArray can only have numbers for content. Any program that is sharing hardened TypedArrays between co-tentant programs and relying on harden to only allow these programs to communicate exclusively by changing numbers within the bounds of the TypedArray, may inadvertently have arranged for a mechanism for a pair of third-parties to communicate arbitrary objects on these other properties.

Patches

Has the problem been patched? What versions should users upgrade to?

SES version 0.16.0 patches this issue, causing harden to recognize properties with non-canonical numeric representations and ensuring that these properties are non-configurable.

Workarounds

Is there a way for users to fix or remediate the vulnerability without upgrading?

Users should avoid sharing TypedArrays between co-tenant programs and instead create wrapper objects that produce a read-only view of the underlying data. We allow harden to succeed for TypedArrays because the treatment is in fact consistent with the behavior of collections like Map, but all collections shared between co-tentant programs should probably be attenuated to either read- or write-only facets and probably close over only part of the content of the collection. However, the motivation for allowing TypedArrays to be hardened in practice is to allow certain legacy modules to function under Hardened JavaScript with LavaMoat, since they export TypedArrays, even though they would ideally export read-only facets of these.

References

Are there any links users can visit to find out more?

Not at this time.

For more information

If you have any questions or comments about this advisory:

• Email us at security@agoric.com