Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Font-family variables that can be used to customise the sans-serif and monospace fonts used in the editor (#1264)
- Material symbols font to web component preview page since the Design System depends on this (#1261)
- Ability for plugins to add buttons to the SidebarPanel header (#1270, #1271, #1274)
- Prevent access to the session from within the editor (#1275)

### Changed

Expand Down
125 changes: 80 additions & 45 deletions src/components/Editor/Runners/HtmlRunner/HtmlRunner.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -306,55 +306,90 @@ function HtmlRunner() {
if (!externalLink) {
const indexPage = parse(focussedComponent(previewFile).content);
const body = indexPage.querySelector("body") || indexPage;
const htmlRoot = indexPage.querySelector("html") ?? indexPage;

// insert script to disable access to specific localStorage keys
// localstorage.getItem() is a potential security risk when executing untrusted code
const disableLocalStorageScript = `
<script>
(function() {
const originalGetItem = window.localStorage.getItem.bind(window.localStorage);
const originalSetItem = window.localStorage.setItem.bind(window.localStorage);
const originalRemoveItem = window.localStorage.removeItem.bind(window.localStorage);
const originalClear = window.localStorage.clear.bind(window.localStorage);

const isDisallowedKey = (key) => key === 'authKey' || key.startsWith('oidc.');

Object.defineProperty(window, 'localStorage', {
value: {
getItem: function(key) {
if (isDisallowedKey(key)) {
console.log(\`localStorage.getItem for "\${key}" is disabled\`);
return null;
}
return originalGetItem(key);
},
setItem: function(key, value) {
if (isDisallowedKey(key)) {
console.log(\`localStorage.setItem for "\${key}" is disabled\`);
return;
}
return originalSetItem(key, value);
},
removeItem: function(key) {
if (isDisallowedKey(key)) {
console.log(\`localStorage.removeItem for "\${key}" is disabled\`);
return;
}
return originalRemoveItem(key);
<script>
(function () {
"use strict";
const isBlocked = (key) =>
typeof key === "string" && (key === "authKey" || key.startsWith("oidc."));
const wrapLocal = (storage) =>
storage && {
getItem(key) {
return isBlocked(key) ? null : storage.getItem(key);
},
setItem(key, value) {
if (!isBlocked(key)) storage.setItem(key, value);
},
removeItem(key) {
if (!isBlocked(key)) storage.removeItem(key);
},
clear() {},
key(index) {
const name = storage.key(index);
return isBlocked(name) ? null : name;
},
get length() {
return storage?.length ?? 0;
},
};
const apply = (host) => {
if (!host) return;
try {
const guarded = wrapLocal(host.localStorage);
if (!guarded) return;
Object.defineProperty(host, "localStorage", {
configurable: false,
enumerable: false,
get: () => guarded,
set: () => undefined,
});
} catch (_) {
/* ignore if non-configurable */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this helpful or can it be dropped?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... or is there any value in raising something?

}
};
[window, window.parent, window.top, document.defaultView].forEach(apply);
})();
</script>
`;

const disableSessionStorageScript = `
<script>
(function () {
"use strict";
const stub = {
getItem: () => null,
setItem: () => undefined,
removeItem: () => undefined,
clear: () => undefined,
key: () => null,
get length() {
return 0;
},
clear: function() {
console.log('localStorage.clear is disabled');
return;
};
const apply = (host) => {
if (!host) return;
try {
Object.defineProperty(host, "sessionStorage", {
configurable: false,
enumerable: false,
get: () => stub,
set: () => undefined,
});
} catch (_) {
/* ignore if non-configurable */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

}
},
writable: false,
configurable: false
});
})();
</script>
`;

body.insertAdjacentHTML("afterbegin", disableLocalStorageScript);
};
[window, window.parent, window.top, document.defaultView].forEach(apply);
})();
</script>
`;

// insert scripts to disable access to specific localStorage keys and sessionStorage
// entirely, they are both potential security risks when executing untrusted code
htmlRoot.insertAdjacentHTML("afterbegin", disableLocalStorageScript);
htmlRoot.insertAdjacentHTML("afterbegin", disableSessionStorageScript);

replaceHrefNodes(indexPage, projectCode);
replaceSrcNodes(indexPage, projectMedia, projectCode);
Expand Down
28 changes: 22 additions & 6 deletions src/components/Editor/Runners/HtmlRunner/HtmlRunner.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -289,19 +289,35 @@ describe("When run is triggered", () => {
const [generatedHtml] = Blob.mock.calls[0][0];

expect(generatedHtml).toContain("<script>");
expect(generatedHtml).toContain("const isBlocked = (key) =>");
expect(generatedHtml).toContain(
"Object.defineProperty(window, 'localStorage'",
'typeof key === "string" && (key === "authKey" || key.startsWith("oidc."));',
);
expect(generatedHtml).toContain("getItem: function(key) {");
expect(generatedHtml).toContain(
'Object.defineProperty(host, "localStorage"',
);
expect(generatedHtml).toContain("getItem(key) {");
expect(generatedHtml).toContain(
"return isBlocked(key) ? null : storage.getItem(key);",
);
expect(generatedHtml).toContain(
"[window, window.parent, window.top, document.defaultView].forEach(apply);",
);
expect(generatedHtml).toContain("</script>");
});

test("Includes localSession disabling script to prevent all access to the session object", () => {
const [generatedHtml] = Blob.mock.calls[0][0];

expect(generatedHtml).toContain("<script>");
expect(generatedHtml).toContain(
"const isDisallowedKey = (key) => key === 'authKey' || key.startsWith('oidc.');",
'Object.defineProperty(host, "sessionStorage"',
);
expect(generatedHtml).toContain("if (isDisallowedKey(key))");
expect(generatedHtml).toContain("get: () => stub");
expect(generatedHtml).toContain("set: () => undefined");
expect(generatedHtml).toContain(
'localStorage.getItem for "${key}" is disabled', // eslint-disable-line no-template-curly-in-string
"[window, window.parent, window.top, document.defaultView].forEach(apply);",
);
expect(generatedHtml).toContain("return null;");
expect(generatedHtml).toContain("</script>");
});
});
Expand Down
Loading