Skip to content

Commit ed90fd8

Browse files
authored
Disable specific localstorage access at runtime (#1079)
Accessing localstorage (eg. `localstorage.getItem('authKey')`) is a potential security risk when running code that has been shared between individuals / is untrusted. This change overrides the specific localstorage method / key access and provides feedback in the browser console. Access to localstorage is not entirely blocked as it is used in many projects / resources eg. [Share Your World](https://projects.raspberrypi.org/en/projects/share-your-world) (https://github.com/search?q=repo%3Araspberrypilearning%2Fshare-your-world%20localstorage&type=code) For more context, see: https://github.com/RaspberryPiFoundation/documentation/pull/112/files *** The change can be tested by creating a new HTML project in the editor, inputting the following and inspecting the console for the output: ``` <script> localStorage.setItem("foo", "bar") console.log(localStorage.getItem("foo")) localStorage.setItem("authKey", "secret") console.log(localStorage.getItem("authKey")) </script> ``` As demonstrated here: ![Screenshot 2024-09-25 at 11 47 54](https://github.com/user-attachments/assets/c145fc04-e2c2-4df6-bb87-5d88ee8ea5e5)
1 parent 19b74d7 commit ed90fd8

File tree

5 files changed

+163
-28
lines changed

5 files changed

+163
-28
lines changed

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file.
44

55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
66

7+
## Unreleased
8+
9+
### Added
10+
11+
- Disabling of `localstorage.getItem()` access to `authKey`, `oidc.user:https://staging-auth-v1.raspberrypi.org:editor-api` and `oidc.user:https://auth-v1.raspberrypi.org:editor-api` at runtime (#1079)
12+
713
## [0.26.0] - 2024-09-13
814

915
### Added

README.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,11 @@ See the section about [deployment](https://facebook.github.io/create-react-app/d
5656

5757
Automated unit tests can be run via the `yarn test` command. These unit tests are written using the JavaScript testing framework `Jest` and make use of the tools provided by the [React Testing Library](https://testing-library.com/docs/). Automated accessibility testing for components is available via the `jest-axe` library. This can be achieved using the `haveNoViolations` matcher provided by `jest-axe`, although this does not guarantee that the tested components have no accessibility issues.
5858

59-
Integration testing is carried out via `cypress` and can be run using the `yarn exec cypress run` commmand. Currently, there are basic `cypress` tests for the standalone editor site, the web component and Mission Zero-related functionality. These can be found in the `cypress/e2e` directory. Screenshots and videos related to the most recent `cypress` test run can be found in `cypress/screenshots` and `cypress/videos` respectively.
59+
Integration testing is carried out via `cypress` and can be run using:
60+
* `yarn exec cypress run` to run in the CLI
61+
* `yarn exec cypress open` to run in the GUI
62+
63+
Currently, there are basic `cypress` tests for the standalone editor site, the web component and Mission Zero-related functionality. These can be found in the `cypress/e2e` directory. Screenshots and videos related to the most recent `cypress` test run can be found in `cypress/screenshots` and `cypress/videos` respectively.
6064

6165
## Web Component
6266

cypress/e2e/spec-html.cy.js

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,46 @@ const makeNewFile = (filename = "new.html") => {
2020
cy.get("div[class=modal-content__buttons]").contains("Add file").click();
2121
};
2222

23+
it("blocks access to specific localStorage keys but allows other keys", () => {
24+
localStorage.clear();
25+
localStorage.setItem("foo", "bar");
26+
localStorage.setItem("authKey", "secret");
27+
localStorage.setItem(
28+
"oidc.user:https://staging-auth-v1.raspberrypi.org:editor-api",
29+
"staging-token",
30+
);
31+
localStorage.setItem(
32+
"oidc.user:https://auth-v1.raspberrypi.org:editor-api",
33+
"prod-token",
34+
);
35+
36+
cy.visit(baseUrl);
37+
cy.get(".btn--run").click();
38+
cy.get("iframe#output-frame").should("exist");
39+
40+
cy.get("iframe#output-frame").then(($iframe) => {
41+
const iframeWindow = $iframe[0].contentWindow;
42+
43+
cy.wrap(iframeWindow).then((win) => {
44+
const authKeyResult = win.localStorage.getItem("authKey");
45+
expect(authKeyResult).to.equal(null);
46+
47+
const stagingOidcResult = win.localStorage.getItem(
48+
"oidc.user:https://staging-auth-v1.raspberrypi.org:editor-api",
49+
);
50+
expect(stagingOidcResult).to.equal(null);
51+
52+
const prodOidcResult = win.localStorage.getItem(
53+
"oidc.user:https://auth-v1.raspberrypi.org:editor-api",
54+
);
55+
expect(prodOidcResult).to.equal(null);
56+
57+
const fooResult = win.localStorage.getItem("foo");
58+
expect(fooResult).to.equal("bar");
59+
});
60+
});
61+
});
62+
2363
it("renders the html runner", () => {
2464
cy.visit(baseUrl);
2565
cy.get(".btn--run").click();

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

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,55 @@ function HtmlRunner() {
297297
const indexPage = parse(focussedComponent(previewFile).content);
298298
const body = indexPage.querySelector("body") || indexPage;
299299

300+
// insert script to disable access to specific localStorage keys
301+
// localstorage.getItem() is a potential security risk when executing untrusted code
302+
const disableLocalStorageScript = `
303+
<script>
304+
(function() {
305+
const disallowedKeys = ['authKey', 'oidc.user:https://staging-auth-v1.raspberrypi.org:editor-api', 'oidc.user:https://auth-v1.raspberrypi.org:editor-api'];
306+
307+
const originalGetItem = window.localStorage.getItem.bind(window.localStorage);
308+
const originalSetItem = window.localStorage.setItem.bind(window.localStorage);
309+
const originalRemoveItem = window.localStorage.removeItem.bind(window.localStorage);
310+
const originalClear = window.localStorage.clear.bind(window.localStorage);
311+
312+
Object.defineProperty(window, 'localStorage', {
313+
value: {
314+
getItem: function(key) {
315+
if (disallowedKeys.includes(key)) {
316+
console.log(\`localStorage.getItem for "\${key}" is disabled\`);
317+
return null;
318+
}
319+
return originalGetItem(key);
320+
},
321+
setItem: function(key, value) {
322+
if (disallowedKeys.includes(key)) {
323+
console.log(\`localStorage.setItem for "\${key}" is disabled\`);
324+
return;
325+
}
326+
return originalSetItem(key, value);
327+
},
328+
removeItem: function(key) {
329+
if (disallowedKeys.includes(key)) {
330+
console.log(\`localStorage.removeItem for "\${key}" is disabled\`);
331+
return;
332+
}
333+
return originalRemoveItem(key);
334+
},
335+
clear: function() {
336+
console.log('localStorage.clear is disabled');
337+
return;
338+
}
339+
},
340+
writable: false,
341+
configurable: false
342+
});
343+
})();
344+
</script>
345+
`;
346+
347+
body.insertAdjacentHTML("afterbegin", disableLocalStorageScript);
348+
300349
replaceHrefNodes(indexPage, projectCode);
301350
replaceSrcNodes(indexPage, projectImages, projectCode);
302351
replaceSrcNodes(indexPage, projectImages, projectCode, "data-src");

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

Lines changed: 63 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -273,25 +273,43 @@ describe("When run is triggered", () => {
273273
});
274274

275275
test("Runs HTML code and adds meta tag", () => {
276-
const indexPageContent =
277-
'<head></head><body><p>hello world</p><meta filename="index.html" ></body>';
278-
expect(Blob).toHaveBeenCalledWith([indexPageContent], {
279-
type: "text/html",
280-
});
276+
const [generatedHtml] = Blob.mock.calls[0][0];
277+
278+
expect(generatedHtml).toContain("<p>hello world</p>");
279+
expect(generatedHtml).toContain('<meta filename="index.html"');
281280
});
282281

283282
test("Dispatches action to end code run", () => {
284283
expect(store.getActions()).toEqual(
285284
expect.arrayContaining([codeRunHandled()]),
286285
);
287286
});
287+
288+
test("Includes localStorage disabling script for disallowed keys in the iframe", () => {
289+
const [generatedHtml] = Blob.mock.calls[0][0];
290+
291+
expect(generatedHtml).toContain("<script>");
292+
expect(generatedHtml).toContain(
293+
"Object.defineProperty(window, 'localStorage'",
294+
);
295+
expect(generatedHtml).toContain("getItem: function(key) {");
296+
297+
expect(generatedHtml).toContain(
298+
"const disallowedKeys = ['authKey', 'oidc.user:https://staging-auth-v1.raspberrypi.org:editor-api', 'oidc.user:https://auth-v1.raspberrypi.org:editor-api']",
299+
);
300+
expect(generatedHtml).toContain("if (disallowedKeys.includes(key))");
301+
expect(generatedHtml).toContain(
302+
'localStorage.getItem for "${key}" is disabled',
303+
);
304+
expect(generatedHtml).toContain("return null;");
305+
expect(generatedHtml).toContain("</script>");
306+
});
288307
});
289308

290309
describe("When a non-permitted external link is rendered", () => {
291310
let store;
292311
const input =
293312
'<head></head><body><a href="https://google.test/">EXTERNAL LINK!</a></body>';
294-
const output = `<head></head><body><a href="javascript:void(0)" onclick="window.parent.postMessage({msg: 'ERROR: External link'})">EXTERNAL LINK!</a><meta filename="index.html" ></body>`;
295313

296314
beforeEach(() => {
297315
const middlewares = [];
@@ -326,18 +344,22 @@ describe("When a non-permitted external link is rendered", () => {
326344
);
327345
});
328346

329-
test("Runs HTML code without the link", () => {
330-
expect(Blob).toHaveBeenCalledWith([output], {
331-
type: "text/html",
332-
});
347+
test("Transforms the external link and includes the meta tag", () => {
348+
const [generatedHtml] = Blob.mock.calls[0][0];
349+
350+
expect(generatedHtml).toContain('<a href="javascript:void(0)"');
351+
expect(generatedHtml).toContain(
352+
"onclick=\"window.parent.postMessage({msg: 'ERROR: External link'})\"",
353+
);
354+
expect(generatedHtml).toContain("EXTERNAL LINK!");
355+
expect(generatedHtml).toContain('<meta filename="index.html"');
333356
});
334357
});
335358

336359
describe("When a new tab link is rendered", () => {
337360
let store;
338361
const input =
339362
'<head></head><body><a href="index.html" target="_blank">NEW TAB LINK!</a></body>';
340-
const output = `<head></head><body><a href="javascript:void(0)" onclick="window.parent.postMessage({msg: 'RELOAD', payload: { linkTo: 'index' }})">NEW TAB LINK!</a><meta filename="some_file.html" ></body>`;
341363

342364
beforeEach(() => {
343365
const middlewares = [];
@@ -373,16 +395,21 @@ describe("When a new tab link is rendered", () => {
373395
);
374396
});
375397

376-
test("Runs HTML code removes target attribute", () => {
377-
expect(Blob).toHaveBeenCalledWith([output], {
378-
type: "text/html",
379-
});
398+
test("Removes target attribute and adds onclick event", () => {
399+
const [generatedHtml] = Blob.mock.calls[0][0];
400+
401+
expect(generatedHtml).not.toContain('target="_blank"');
402+
expect(generatedHtml).toContain('<a href="javascript:void(0)"');
403+
expect(generatedHtml).toContain(
404+
"onclick=\"window.parent.postMessage({msg: 'RELOAD', payload: { linkTo: 'index' }})\"",
405+
);
406+
expect(generatedHtml).toContain("NEW TAB LINK!");
407+
expect(generatedHtml).toContain('<meta filename="some_file.html"');
380408
});
381409
});
382410

383411
describe("When an internal link is rendered", () => {
384412
let store;
385-
const output = `<head></head><body><a href="javascript:void(0)" onclick="window.parent.postMessage({msg: 'RELOAD', payload: { linkTo: 'test' }})">ANCHOR LINK!</a><meta filename="internal_link.html" ></body>`;
386413
beforeEach(() => {
387414
const middlewares = [];
388415
const mockStore = configureStore(middlewares);
@@ -417,18 +444,20 @@ describe("When an internal link is rendered", () => {
417444
);
418445
});
419446

420-
test("Runs HTML code without changes apart from meta tag", () => {
421-
expect(Blob).toHaveBeenCalledWith([output], {
422-
type: "text/html",
423-
});
447+
test("Transforms internal link and includes meta tag", () => {
448+
const [generatedHtml] = Blob.mock.calls[0][0];
449+
450+
expect(generatedHtml).toContain('<a href="javascript:void(0)"');
451+
expect(generatedHtml).toContain(
452+
"onclick=\"window.parent.postMessage({msg: 'RELOAD', payload: { linkTo: 'test' }})\"",
453+
);
454+
expect(generatedHtml).toContain("ANCHOR LINK!");
455+
expect(generatedHtml).toContain('<meta filename="internal_link.html"');
424456
});
425457
});
426458

427459
describe("When an allowed external link is rendered", () => {
428460
let store;
429-
430-
const output = `<head></head><body><a href="https://rpf.io/seefood" onclick="window.parent.postMessage({msg: 'Allowed external link', payload: { linkTo: 'https://rpf.io/seefood' }})">RPF link</a><meta filename="allowed_external_link.html" ></body>`;
431-
432461
beforeEach(() => {
433462
const middlewares = [];
434463
const mockStore = configureStore(middlewares);
@@ -456,10 +485,17 @@ describe("When an allowed external link is rendered", () => {
456485
);
457486
});
458487

459-
test("Runs HTML code with the link included", () => {
460-
expect(Blob).toHaveBeenCalledWith([output], {
461-
type: "text/html",
462-
});
488+
test("Transforms allowed external link and includes meta tag", () => {
489+
const [generatedHtml] = Blob.mock.calls[0][0];
490+
491+
expect(generatedHtml).toContain('<a href="https://rpf.io/seefood"');
492+
expect(generatedHtml).toContain(
493+
"onclick=\"window.parent.postMessage({msg: 'Allowed external link', payload: { linkTo: 'https://rpf.io/seefood' }})\"",
494+
);
495+
expect(generatedHtml).toContain("RPF link");
496+
expect(generatedHtml).toContain(
497+
'<meta filename="allowed_external_link.html"',
498+
);
463499
});
464500
});
465501

0 commit comments

Comments
 (0)