Skip to content

Commit 30efd38

Browse files
authored
1004: Prevent access to the session across all scopes, and tighten localStorage access scope (#1275)
- Prevent access to the session across all scopes - Tighten localStorage access scope
1 parent d3f0563 commit 30efd38

File tree

3 files changed

+100
-52
lines changed

3 files changed

+100
-52
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
1111
- Font-family variables that can be used to customise the sans-serif and monospace fonts used in the editor (#1264)
1212
- Material symbols font to web component preview page since the Design System depends on this (#1261)
1313
- Ability for plugins to add buttons to the SidebarPanel header (#1270, #1271, #1274)
14+
- Prevent access to the session from within the editor (#1275)
1415

1516
### Changed
1617

src/components/Editor/Runners/HtmlRunner/HtmlRunner.jsx

Lines changed: 77 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -306,55 +306,86 @@ function HtmlRunner() {
306306
if (!externalLink) {
307307
const indexPage = parse(focussedComponent(previewFile).content);
308308
const body = indexPage.querySelector("body") || indexPage;
309+
const htmlRoot = indexPage.querySelector("html") ?? indexPage;
309310

310-
// insert script to disable access to specific localStorage keys
311-
// localstorage.getItem() is a potential security risk when executing untrusted code
312311
const disableLocalStorageScript = `
313-
<script>
314-
(function() {
315-
const originalGetItem = window.localStorage.getItem.bind(window.localStorage);
316-
const originalSetItem = window.localStorage.setItem.bind(window.localStorage);
317-
const originalRemoveItem = window.localStorage.removeItem.bind(window.localStorage);
318-
const originalClear = window.localStorage.clear.bind(window.localStorage);
319-
320-
const isDisallowedKey = (key) => key === 'authKey' || key.startsWith('oidc.');
321-
322-
Object.defineProperty(window, 'localStorage', {
323-
value: {
324-
getItem: function(key) {
325-
if (isDisallowedKey(key)) {
326-
console.log(\`localStorage.getItem for "\${key}" is disabled\`);
327-
return null;
328-
}
329-
return originalGetItem(key);
312+
<script>
313+
(function () {
314+
"use strict";
315+
const isBlocked = (key) =>
316+
typeof key === "string" && (key === "authKey" || key.startsWith("oidc."));
317+
const wrapLocal = (storage) =>
318+
storage && {
319+
getItem(key) {
320+
return isBlocked(key) ? null : storage.getItem(key);
321+
},
322+
setItem(key, value) {
323+
if (!isBlocked(key)) storage.setItem(key, value);
324+
},
325+
removeItem(key) {
326+
if (!isBlocked(key)) storage.removeItem(key);
327+
},
328+
clear() {},
329+
key(index) {
330+
const name = storage.key(index);
331+
return isBlocked(name) ? null : name;
332+
},
333+
get length() {
334+
return storage?.length ?? 0;
335+
},
336+
};
337+
const apply = (host) => {
338+
if (!host) return;
339+
try {
340+
const guarded = wrapLocal(host.localStorage);
341+
if (!guarded) return;
342+
Object.defineProperty(host, "localStorage", {
343+
configurable: false,
344+
enumerable: false,
345+
get: () => guarded,
346+
set: () => undefined,
347+
});
348+
} catch (_) {}
349+
};
350+
[window, window.parent, window.top, document.defaultView].forEach(apply);
351+
})();
352+
</script>
353+
`;
354+
355+
const disableSessionStorageScript = `
356+
<script>
357+
(function () {
358+
"use strict";
359+
const stub = {
360+
getItem: () => null,
361+
setItem: () => undefined,
362+
removeItem: () => undefined,
363+
clear: () => undefined,
364+
key: () => null,
365+
get length() {
366+
return 0;
330367
},
331-
setItem: function(key, value) {
332-
if (isDisallowedKey(key)) {
333-
console.log(\`localStorage.setItem for "\${key}" is disabled\`);
334-
return;
335-
}
336-
return originalSetItem(key, value);
337-
},
338-
removeItem: function(key) {
339-
if (isDisallowedKey(key)) {
340-
console.log(\`localStorage.removeItem for "\${key}" is disabled\`);
341-
return;
342-
}
343-
return originalRemoveItem(key);
344-
},
345-
clear: function() {
346-
console.log('localStorage.clear is disabled');
347-
return;
348-
}
349-
},
350-
writable: false,
351-
configurable: false
352-
});
353-
})();
354-
</script>
355-
`;
356-
357-
body.insertAdjacentHTML("afterbegin", disableLocalStorageScript);
368+
};
369+
const apply = (host) => {
370+
if (!host) return;
371+
try {
372+
Object.defineProperty(host, "sessionStorage", {
373+
configurable: false,
374+
enumerable: false,
375+
get: () => stub,
376+
set: () => undefined,
377+
});
378+
} catch (_) {}
379+
};
380+
[window, window.parent, window.top, document.defaultView].forEach(apply);
381+
})();
382+
</script>
383+
`;
384+
385+
// insert scripts to disable access to specific localStorage keys and sessionStorage
386+
// entirely, they are both potential security risks when executing untrusted code
387+
htmlRoot.insertAdjacentHTML("afterbegin", disableLocalStorageScript);
388+
htmlRoot.insertAdjacentHTML("afterbegin", disableSessionStorageScript);
358389

359390
replaceHrefNodes(indexPage, projectCode);
360391
replaceSrcNodes(indexPage, projectMedia, projectCode);

src/components/Editor/Runners/HtmlRunner/HtmlRunner.test.js

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -289,19 +289,35 @@ describe("When run is triggered", () => {
289289
const [generatedHtml] = Blob.mock.calls[0][0];
290290

291291
expect(generatedHtml).toContain("<script>");
292+
expect(generatedHtml).toContain("const isBlocked = (key) =>");
292293
expect(generatedHtml).toContain(
293-
"Object.defineProperty(window, 'localStorage'",
294+
'typeof key === "string" && (key === "authKey" || key.startsWith("oidc."));',
294295
);
295-
expect(generatedHtml).toContain("getItem: function(key) {");
296+
expect(generatedHtml).toContain(
297+
'Object.defineProperty(host, "localStorage"',
298+
);
299+
expect(generatedHtml).toContain("getItem(key) {");
300+
expect(generatedHtml).toContain(
301+
"return isBlocked(key) ? null : storage.getItem(key);",
302+
);
303+
expect(generatedHtml).toContain(
304+
"[window, window.parent, window.top, document.defaultView].forEach(apply);",
305+
);
306+
expect(generatedHtml).toContain("</script>");
307+
});
296308

309+
test("Includes localSession disabling script to prevent all access to the session object", () => {
310+
const [generatedHtml] = Blob.mock.calls[0][0];
311+
312+
expect(generatedHtml).toContain("<script>");
297313
expect(generatedHtml).toContain(
298-
"const isDisallowedKey = (key) => key === 'authKey' || key.startsWith('oidc.');",
314+
'Object.defineProperty(host, "sessionStorage"',
299315
);
300-
expect(generatedHtml).toContain("if (isDisallowedKey(key))");
316+
expect(generatedHtml).toContain("get: () => stub");
317+
expect(generatedHtml).toContain("set: () => undefined");
301318
expect(generatedHtml).toContain(
302-
'localStorage.getItem for "${key}" is disabled', // eslint-disable-line no-template-curly-in-string
319+
"[window, window.parent, window.top, document.defaultView].forEach(apply);",
303320
);
304-
expect(generatedHtml).toContain("return null;");
305321
expect(generatedHtml).toContain("</script>");
306322
});
307323
});

0 commit comments

Comments
 (0)