diff --git a/packages/rollup-plugin-webbundle/test/test.js b/packages/rollup-plugin-webbundle/test/test.js index 92d593b..28e7537 100644 --- a/packages/rollup-plugin-webbundle/test/test.js +++ b/packages/rollup-plugin-webbundle/test/test.js @@ -15,7 +15,7 @@ */ import test from 'ava'; -import * as fs from 'fs'; +import fs from 'fs'; import * as os from 'os'; import * as path from 'path'; import * as rollup from 'rollup'; @@ -182,11 +182,12 @@ test('static with symbolic link throws error', async (t) => { } }); -test('static with directory-symlink loop throws refusal error without ELOOP', async (t) => { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'wbn-rollup-loop-')); +test('static with scheme-like directory refuses foreign origin', async (t) => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'wbn-rollup-foreign-')); try { - const loopLink = path.join(tmpDir, 'loop'); - fs.symlinkSync('.', loopLink); + const foreignDir = path.join(tmpDir, 'https:', 'attacker.test'); + fs.mkdirSync(foreignDir, { recursive: true }); + fs.writeFileSync(path.join(foreignDir, 'p.js'), 'alert(1);'); const bundle = await rollup.rollup({ input: 'fixtures/index.js', @@ -206,7 +207,7 @@ test('static with directory-symlink loop throws refusal error without ELOOP', as ); t.is( error.message, - `Refusing to bundle symbolic link at ${loopLink}. Replace it with a regular file or directory.` + 'Refusing to add exchange with unexpected origin: https:/attacker.test/p.js' ); } finally { fs.rmSync(tmpDir, { recursive: true, force: true }); diff --git a/packages/shared/test/test.js b/packages/shared/test/test.js index 864553f..3aba932 100644 --- a/packages/shared/test/test.js +++ b/packages/shared/test/test.js @@ -20,7 +20,11 @@ import * as os from 'os'; import * as path from 'path'; import { BundleBuilder } from 'wbn'; import { getValidatedOptionsWithDefaults } from '../lib/types.js'; -import { addFilesRecursively } from '../lib/utils.js'; +import { + addAsset, + addFilesRecursively, + assertExchangeUrlOrigin, +} from '../lib/utils.js'; import * as wbnSign from 'wbn-sign'; const TEST_ED25519_PRIVATE_KEY = wbnSign.parsePemKey( @@ -144,3 +148,79 @@ test('addFilesRecursively - refuses root directory being a symbolic link', (t) = fs.rmSync(tmpDir, { recursive: true, force: true }); } }); + +test('addAsset - refuses absolute exchange URL when baseURL is empty', (t) => { + const builder = new BundleBuilder(); + const error = t.throws( + () => { + addAsset( + builder, + '', + 'https:/attacker.test/p.js', + 'console.log("evil");', + { output: 'out.wbn' } + ); + }, + { instanceOf: Error } + ); + t.is( + error.message, + 'Refusing to add exchange with unexpected origin: https:/attacker.test/p.js' + ); +}); + +test('assertExchangeUrlOrigin - throws when origins differ', (t) => { + const errorHttps = t.throws( + () => { + assertExchangeUrlOrigin( + 'https://attacker.test/p.js', + 'https://example.com/' + ); + }, + { instanceOf: Error } + ); + t.is( + errorHttps.message, + 'Refusing to add exchange with unexpected origin: https://attacker.test/p.js' + ); + + const errorIwa = t.throws( + () => { + assertExchangeUrlOrigin( + 'isolated-app://attacker/p.js', + 'isolated-app://legit/' + ); + }, + { instanceOf: Error } + ); + t.is( + errorIwa.message, + 'Refusing to add exchange with unexpected origin: isolated-app://attacker/p.js' + ); +}); + +test('addAsset - allows same origin exchange URL', (t) => { + const builder = new BundleBuilder(); + t.notThrows(() => { + addAsset( + builder, + 'https://example.com/', + 'script.js', + 'console.log("ok");', + { baseURL: 'https://example.com/', output: 'out.wbn' } + ); + }); +}); + +test('addAsset - allows relative exchange URL when baseURL is empty or relative', (t) => { + const builder = new BundleBuilder(); + t.notThrows(() => { + addAsset(builder, '', 'script.js', 'console.log("ok");', { + output: 'out.wbn', + }); + addAsset(builder, '/', 'script.js', 'console.log("ok");', { + baseURL: '/', + output: 'out.wbn', + }); + }); +}); diff --git a/packages/shared/utils.ts b/packages/shared/utils.ts index cfeb957..95f3bf8 100644 --- a/packages/shared/utils.ts +++ b/packages/shared/utils.ts @@ -16,12 +16,48 @@ import * as fs from 'fs'; import * as path from 'path'; +import { URL } from 'url'; import mime from 'mime'; import { combineHeadersForUrl, BundleBuilder } from 'wbn'; import { IntegrityBlockSigner } from 'wbn-sign'; import { checkAndAddIwaHeaders } from './iwa-headers.js'; import { ValidIbSignPluginOptions, ValidPluginOptions } from './types.js'; +// Exchange URLs are built by string-concatenating `baseURL` with a path derived +// from filesystem names or bundler chunk names. POSIX path components may +// legally contain ':', so a crafted directory tree such as `https:/evil/x.js` +// can turn the concatenated result into an absolute URL for a foreign origin. +// Reject any exchange URL that does not stay on `baseURL`'s origin (or, when +// `baseURL` is empty or relative, any exchange URL that parses as absolute at all). +export function assertExchangeUrlOrigin(exchangeUrl: string, baseURL: string) { + let parsed: URL; + try { + parsed = new URL(exchangeUrl); + } catch { + return; // Not an absolute URL; nothing to check. + } + + let baseParsed: URL; + try { + baseParsed = new URL(baseURL); + } catch { + // baseURL is not an absolute URL, so exchangeUrl should not be absolute either. + throw new Error( + `Refusing to add exchange with unexpected origin: ${exchangeUrl}` + ); + } + + if ( + parsed.protocol !== baseParsed.protocol || + parsed.host !== baseParsed.host || + parsed.port !== baseParsed.port + ) { + throw new Error( + `Refusing to add exchange with unexpected origin: ${exchangeUrl}` + ); + } +} + // If the file name is 'index.html', create an entry for both baseURL/dir/ and // baseURL/dir/index.html which redirects to the aforementioned. Otherwise just // for the asset itself. This matches the behavior of gen-bundle. @@ -49,6 +85,7 @@ export function addAsset( ); if (shouldCheckIwaHeaders) checkAndAddIwaHeaders(combinedIndexHeaders); + assertExchangeUrlOrigin(baseURL + relativeAssetPath, baseURL); builder.addExchange( baseURL + relativeAssetPath, 301, @@ -69,6 +106,7 @@ export function addAsset( ); if (shouldCheckIwaHeaders) checkAndAddIwaHeaders(combinedHeaders); + assertExchangeUrlOrigin(baseURLWithAssetPath, baseURL); builder.addExchange( baseURLWithAssetPath, 200, diff --git a/packages/webbundle-webpack-plugin/test/test.js b/packages/webbundle-webpack-plugin/test/test.js index 8c66870..35b1255 100644 --- a/packages/webbundle-webpack-plugin/test/test.js +++ b/packages/webbundle-webpack-plugin/test/test.js @@ -130,6 +130,32 @@ test('static with symbolic link throws error', async (t) => { } }); +test('static with scheme-like directory refuses foreign origin', async (t) => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'wbn-webpack-foreign-')); + try { + const foreignDir = path.join(tmpDir, 'https:', 'attacker.test'); + fs.mkdirSync(foreignDir, { recursive: true }); + fs.writeFileSync(path.join(foreignDir, 'p.js'), 'alert(1);'); + + const error = await t.throwsAsync( + async () => { + await run({ + output: 'example.wbn', + static: { dir: tmpDir }, + }); + }, + { instanceOf: Error } + ); + t.true( + error.message.includes( + 'Refusing to add exchange with unexpected origin: https:/attacker.test/p.js' + ) + ); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } +}); + test('relative', async (t) => { const { memfs } = await run({ static: { dir: path.join(__dirname, 'fixtures', 'static') },