Skip to content

Commit 85abedf

Browse files
RafaelGSSRafaelGSS
committedJul 2, 2024
lib,esm: handle bypass network-import via data:
PR-URL: nodejs-private/node-private#522 Backport-PR-URL: nodejs-private/node-private#602 CVE-ID: CVE-2024-22020
·
v18.20.8v18.20.4
1 parent eccd63b commit 85abedf

File tree

3 files changed

+176
-65
lines changed

3 files changed

+176
-65
lines changed
 

Diff for: ‎lib/internal/modules/esm/resolve.js

+15-2
Original file line numberDiff line numberDiff line change
@@ -1142,9 +1142,19 @@ function defaultResolve(specifier, context = {}) {
11421142
} else {
11431143
parsed = new URL(specifier);
11441144
}
1145-
11461145
// Avoid accessing the `protocol` property due to the lazy getters.
11471146
const protocol = parsed.protocol;
1147+
1148+
if (protocol === 'data:' &&
1149+
parsedParentURL.protocol !== 'file:' &&
1150+
experimentalNetworkImports) {
1151+
throw new ERR_NETWORK_IMPORT_DISALLOWED(
1152+
specifier,
1153+
parsedParentURL,
1154+
'import data: from a non file: is not allowed',
1155+
);
1156+
}
1157+
11481158
if (protocol === 'data:' ||
11491159
(experimentalNetworkImports &&
11501160
(
@@ -1155,7 +1165,10 @@ function defaultResolve(specifier, context = {}) {
11551165
) {
11561166
return { __proto__: null, url: parsed.href };
11571167
}
1158-
} catch {
1168+
} catch (e) {
1169+
if (e?.code === 'ERR_NETWORK_IMPORT_DISALLOWED') {
1170+
throw e;
1171+
}
11591172
// Ignore exception
11601173
}
11611174

Diff for: ‎test/es-module/test-http-imports.mjs

+160-63
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,17 @@
11
// Flags: --experimental-network-imports --dns-result-order=ipv4first
22
import * as common from '../common/index.mjs';
3-
import { path, readKey } from '../common/fixtures.mjs';
4-
import { pathToFileURL } from 'url';
3+
import * as fixtures from '../common/fixtures.mjs';
4+
import tmpdir from '../common/tmpdir.js';
55
import assert from 'assert';
66
import http from 'http';
77
import os from 'os';
88
import util from 'util';
9+
import { describe, it } from 'node:test';
910

1011
if (!common.hasCrypto) {
1112
common.skip('missing crypto');
1213
}
14+
tmpdir.refresh();
1315

1416
const https = (await import('https')).default;
1517

@@ -18,8 +20,8 @@ const createHTTPServer = http.createServer;
1820
// Needed to deal w/ test certs
1921
process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0';
2022
const options = {
21-
key: readKey('agent1-key.pem'),
22-
cert: readKey('agent1-cert.pem')
23+
key: fixtures.readKey('agent1-key.pem'),
24+
cert: fixtures.readKey('agent1-cert.pem')
2325
};
2426

2527
const createHTTPSServer = https.createServer.bind(null, options);
@@ -131,79 +133,174 @@ for (const { protocol, createServer } of [
131133
url.href + 'bar/baz.js'
132134
);
133135

134-
const crossProtocolRedirect = new URL(url.href);
135-
crossProtocolRedirect.searchParams.set('redirect', JSON.stringify({
136-
status: 302,
137-
location: 'data:text/javascript,'
138-
}));
139-
await assert.rejects(
140-
import(crossProtocolRedirect.href),
141-
{ code: 'ERR_NETWORK_IMPORT_DISALLOWED' }
142-
);
143-
144-
const deps = new URL(url.href);
145-
deps.searchParams.set('body', `
146-
export {data} from 'data:text/javascript,export let data = 1';
147-
import * as http from ${JSON.stringify(url.href)};
148-
export {http};
149-
`);
150-
const depsNS = await import(deps.href);
151-
assert.strict.deepStrictEqual(Object.keys(depsNS), ['data', 'http']);
152-
assert.strict.equal(depsNS.data, 1);
153-
assert.strict.equal(depsNS.http, ns);
154-
155-
const relativeDeps = new URL(url.href);
156-
relativeDeps.searchParams.set('body', `
157-
import * as http from "./";
158-
export {http};
159-
`);
160-
const relativeDepsNS = await import(relativeDeps.href);
161-
assert.strict.deepStrictEqual(Object.keys(relativeDepsNS), ['http']);
162-
assert.strict.equal(relativeDepsNS.http, ns);
163-
const fileDep = new URL(url.href);
164-
const { href } = pathToFileURL(path('/es-modules/message.mjs'));
165-
fileDep.searchParams.set('body', `
166-
import ${JSON.stringify(href)};
167-
export default 1;`);
168-
await assert.rejects(
169-
import(fileDep.href),
170-
{ code: 'ERR_NETWORK_IMPORT_DISALLOWED' }
171-
);
172-
173-
const builtinDep = new URL(url.href);
174-
builtinDep.searchParams.set('body', `
175-
import 'node:fs';
176-
export default 1;
177-
`);
178-
await assert.rejects(
179-
import(builtinDep.href),
180-
{ code: 'ERR_NETWORK_IMPORT_DISALLOWED' }
181-
);
182-
183-
const unprefixedBuiltinDep = new URL(url.href);
184-
unprefixedBuiltinDep.searchParams.set('body', `
185-
import 'fs';
186-
export default 1;
187-
`);
188-
await assert.rejects(
189-
import(unprefixedBuiltinDep.href),
190-
{ code: 'ERR_NETWORK_IMPORT_DISALLOWED' }
191-
);
192-
193136
const unsupportedMIME = new URL(url.href);
194137
unsupportedMIME.searchParams.set('mime', 'application/node');
195138
unsupportedMIME.searchParams.set('body', '');
196139
await assert.rejects(
197140
import(unsupportedMIME.href),
198141
{ code: 'ERR_UNKNOWN_MODULE_FORMAT' }
199142
);
143+
200144
const notFound = new URL(url.href);
201145
notFound.pathname = '/not-found';
202146
await assert.rejects(
203147
import(notFound.href),
204148
{ code: 'ERR_MODULE_NOT_FOUND' },
205149
);
206150

151+
const jsonUrl = new URL(url.href + 'json');
152+
jsonUrl.searchParams.set('mime', 'application/json');
153+
jsonUrl.searchParams.set('body', '{"x": 1}');
154+
const json = await import(jsonUrl.href, { with: { type: 'json' } });
155+
assert.deepStrictEqual(Object.keys(json), ['default']);
156+
assert.strictEqual(json.default.x, 1);
157+
158+
await describe('guarantee data url will not bypass import restriction', () => {
159+
it('should not be bypassed by cross protocol redirect', async () => {
160+
const crossProtocolRedirect = new URL(url.href);
161+
crossProtocolRedirect.searchParams.set('redirect', JSON.stringify({
162+
status: 302,
163+
location: 'data:text/javascript,'
164+
}));
165+
await assert.rejects(
166+
import(crossProtocolRedirect.href),
167+
{ code: 'ERR_NETWORK_IMPORT_DISALLOWED' }
168+
);
169+
});
170+
171+
it('should not be bypassed by data URL', async () => {
172+
const deps = new URL(url.href);
173+
deps.searchParams.set('body', `
174+
export {data} from 'data:text/javascript,export let data = 1';
175+
import * as http from ${JSON.stringify(url.href)};
176+
export {http};
177+
`);
178+
await assert.rejects(
179+
import(deps.href),
180+
{ code: 'ERR_NETWORK_IMPORT_DISALLOWED' }
181+
);
182+
});
183+
184+
it('should not be bypassed by encodedURI import', async () => {
185+
const deepDataImport = new URL(url.href);
186+
deepDataImport.searchParams.set('body', `
187+
import 'data:text/javascript,import${encodeURIComponent(JSON.stringify('data:text/javascript,import "os"'))}';
188+
`);
189+
await assert.rejects(
190+
import(deepDataImport.href),
191+
{ code: 'ERR_NETWORK_IMPORT_DISALLOWED' }
192+
);
193+
});
194+
195+
it('should not be bypassed by relative deps import', async () => {
196+
const relativeDeps = new URL(url.href);
197+
relativeDeps.searchParams.set('body', `
198+
import * as http from "./";
199+
export {http};
200+
`);
201+
const relativeDepsNS = await import(relativeDeps.href);
202+
assert.strict.deepStrictEqual(Object.keys(relativeDepsNS), ['http']);
203+
assert.strict.equal(relativeDepsNS.http, ns);
204+
});
205+
206+
it('should not be bypassed by file dependency import', async () => {
207+
const fileDep = new URL(url.href);
208+
const { href } = fixtures.fileURL('/es-modules/message.mjs');
209+
fileDep.searchParams.set('body', `
210+
import ${JSON.stringify(href)};
211+
export default 1;`);
212+
await assert.rejects(
213+
import(fileDep.href),
214+
{ code: 'ERR_NETWORK_IMPORT_DISALLOWED' }
215+
);
216+
});
217+
218+
it('should not be bypassed by builtin dependency import', async () => {
219+
const builtinDep = new URL(url.href);
220+
builtinDep.searchParams.set('body', `
221+
import 'node:fs';
222+
export default 1;
223+
`);
224+
await assert.rejects(
225+
import(builtinDep.href),
226+
{ code: 'ERR_NETWORK_IMPORT_DISALLOWED' }
227+
);
228+
});
229+
230+
231+
it('should not be bypassed by unprefixed builtin dependency import', async () => {
232+
const unprefixedBuiltinDep = new URL(url.href);
233+
unprefixedBuiltinDep.searchParams.set('body', `
234+
import 'fs';
235+
export default 1;
236+
`);
237+
await assert.rejects(
238+
import(unprefixedBuiltinDep.href),
239+
{ code: 'ERR_NETWORK_IMPORT_DISALLOWED' }
240+
);
241+
});
242+
243+
it('should not be bypassed by indirect network import', async () => {
244+
const indirect = new URL(url.href);
245+
indirect.searchParams.set('body', `
246+
import childProcess from 'data:text/javascript,export { default } from "node:child_process"'
247+
export {childProcess};
248+
`);
249+
await assert.rejects(
250+
import(indirect.href),
251+
{ code: 'ERR_NETWORK_IMPORT_DISALLOWED' }
252+
);
253+
});
254+
255+
it('data: URL can always import other data:', async () => {
256+
const data = new URL('data:text/javascript,');
257+
data.searchParams.set('body',
258+
'import \'data:text/javascript,import \'data:\''
259+
);
260+
// doesn't throw
261+
const empty = await import(data.href);
262+
assert.ok(empty);
263+
});
264+
265+
it('data: URL cannot import file: or builtin', async () => {
266+
const data1 = new URL(url.href);
267+
data1.searchParams.set('body',
268+
'import \'file:///some/file.js\''
269+
);
270+
await assert.rejects(
271+
import(data1.href),
272+
{ code: 'ERR_NETWORK_IMPORT_DISALLOWED' }
273+
);
274+
275+
const data2 = new URL(url.href);
276+
data2.searchParams.set('body',
277+
'import \'node:fs\''
278+
);
279+
await assert.rejects(
280+
import(data2.href),
281+
{ code: 'ERR_NETWORK_IMPORT_DISALLOWED' }
282+
);
283+
});
284+
285+
it('data: URL cannot import HTTP URLs', async () => {
286+
const module = fixtures.fileURL('/es-modules/import-data-url.mjs');
287+
try {
288+
await import(module);
289+
} catch (err) {
290+
// We only want the module to load, we don't care if the module throws an
291+
// error as long as the loader does not.
292+
assert.notStrictEqual(err?.code, 'ERR_MODULE_NOT_FOUND');
293+
}
294+
const data1 = new URL(url.href);
295+
const dataURL = 'data:text/javascript;export * from "node:os"';
296+
data1.searchParams.set('body', `export * from ${JSON.stringify(dataURL)};`);
297+
await assert.rejects(
298+
import(data1),
299+
{ code: 'ERR_NETWORK_IMPORT_DISALLOWED' }
300+
);
301+
});
302+
});
303+
207304
server.close();
208305
}
209306
}

Diff for: ‎test/fixtures/es-modules/import-data-url.mjs

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
import "data:text/javascript;export * from \"node:os\"";

1 commit comments

Comments
 (1)

eligrey commented on Jul 8, 2024

@eligrey
eligrey
on Jul 8, 2024edited by eligrey

These changes are incorrect. data: URI imports are not network requests. I don't understand why you're making this distinction, especially given that you don't consider file: URIs as network requests.

It's actually quite ironic since you can make network requests directly with file: URIs but not data: URIs (in browsers - no idea about node.js)

Please sign in to comment.