Skip to content

Commit 7d3d648

Browse files
NAME-ASHWANIYADAVNAME-ASHWANIYADAV
authored
fix: replace innerHTML with textContent across core widgets for security (#7757)
* fix: replace innerHTML with textContent in jseditor.js * fix: replace innerHTML with textContent in turtles.js, palette.js, and toolbar-ui.js * test: update toolbar and palette tests to match textContent refactor
1 parent bc4546f commit 7d3d648

6 files changed

Lines changed: 68 additions & 31 deletions

File tree

‎js/__tests__/palette.test.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1516,6 +1516,8 @@ describe("Palettes Class", () => {
15161516
insertRow: jest.fn(() => ({
15171517
style: {},
15181518
innerHTML: "",
1519+
textContent: "",
1520+
appendChild: jest.fn(),
15191521
children: [{ style: {}, appendChild: jest.fn() }]
15201522
}))
15211523
},

‎js/__tests__/toolbar.test.js‎

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,8 @@ describe("Toolbar Class", () => {
182182
test("renderLogoIcon sets up logo with correct interactions", () => {
183183
const elements = {
184184
"mb-logo": {
185-
innerHTML: "",
185+
textContent: "",
186+
appendChild: jest.fn(),
186187
onmouseenter: null,
187188
onmouseleave: null,
188189
onclick: null,
@@ -208,7 +209,7 @@ describe("Toolbar Class", () => {
208209

209210
//Non-Japanese language
210211
toolbar.renderLogoIcon(mockOnClick);
211-
expect(elements["mb-logo"].innerHTML).toBe("");
212+
expect(elements["mb-logo"].textContent).toBe("");
212213
expect(typeof elements["mb-logo"].onmouseenter).toBe("function");
213214
expect(typeof elements["mb-logo"].onmouseleave).toBe("function");
214215
expect(typeof elements["mb-logo"].onclick).toBe("function");
@@ -225,8 +226,7 @@ describe("Toolbar Class", () => {
225226
// Japanese language
226227
toolbar.language = "ja";
227228
toolbar.renderLogoIcon(mockOnClick);
228-
expect(elements["mb-logo"].innerHTML).toContain("logo-ja.svg");
229-
expect(elements["mb-logo"].innerHTML).toContain("transform: scale(0.85)");
229+
expect(elements["mb-logo"].appendChild).toHaveBeenCalled();
230230
elements["mb-logo"].onclick();
231231
expect(mockOnClick).toHaveBeenCalledTimes(2);
232232
});
@@ -385,10 +385,19 @@ describe("Toolbar Class", () => {
385385
});
386386

387387
test("renderThemeSelectIcon sets onclick and updates theme selection", () => {
388-
const themeSelectIcon = { onclick: null };
388+
const themeSelectIcon = {
389+
onclick: null,
390+
textContent: "",
391+
childNodes: [],
392+
appendChild: jest.fn(),
393+
cloneNode: jest.fn()
394+
};
389395
const themes = ["light", "dark"];
390396
const themeBox = { setAttribute: jest.fn() };
391-
global.docById.mockReturnValue(themeSelectIcon);
397+
global.docById.mockImplementation(id => {
398+
if (id === "themeSelectIcon") return themeSelectIcon;
399+
return { childNodes: [], cloneNode: jest.fn() };
400+
});
392401
global.localStorage.themePreference = "light";
393402
toolbar.renderThemeSelectIcon(themeBox, themes);
394403
expect(themeSelectIcon.onclick).toBeInstanceOf(Function);
@@ -576,7 +585,9 @@ describe("Toolbar Class", () => {
576585
const recordButton = {
577586
classList: { add: jest.fn() },
578587
style: { display: "" },
579-
innerHTML: ""
588+
innerHTML: "",
589+
textContent: "",
590+
appendChild: jest.fn()
580591
};
581592
global.docById.mockReturnValue(recordButton);
582593
global.fnBrowserDetect = jest.fn(() => "firefox");
@@ -592,13 +603,17 @@ describe("Toolbar Class", () => {
592603
classList: { add: jest.fn(), remove: jest.fn() },
593604
style: { display: "" },
594605
innerHTML: "",
606+
textContent: "",
607+
appendChild: jest.fn(),
595608
onclick: null
596609
};
597610

598611
const recordDropdownArrow = {
599612
classList: { add: jest.fn(), remove: jest.fn() },
600613
style: { display: "" },
601614
innerHTML: "",
615+
textContent: "",
616+
appendChild: jest.fn(),
602617
addEventListener: jest.fn(),
603618
removeEventListener: jest.fn(),
604619
querySelector: jest.fn(() => ({ textContent: "arrow_drop_down" })),
@@ -728,7 +743,7 @@ describe("Toolbar Class", () => {
728743
clickHandler();
729744

730745
expect(mockOnClick).toHaveBeenCalledWith(toolbar.activity, false);
731-
expect(elements.menu.innerHTML).toBe("more_vert");
746+
expect(elements.menu.textContent).toBe("more_vert");
732747
expect(elements.toggleAuxBtn.classList.add).toHaveBeenCalledWith("blue", "darken-1");
733748
expect(elements.search.classList.toggle).toHaveBeenCalledWith("open");
734749

@@ -737,7 +752,7 @@ describe("Toolbar Class", () => {
737752

738753
expect(mockOnClick).toHaveBeenCalledWith(toolbar.activity, true);
739754
expect(elements["aux-toolbar"].style.display).toBe("none");
740-
expect(elements.menu.innerHTML).toBe("menu");
755+
expect(elements.menu.textContent).toBe("menu");
741756
expect(elements.toggleAuxBtn.classList.remove).toHaveBeenCalledWith("blue", "darken-1");
742757
expect(elements.toggleAuxBtn.className).toBe("tooltipped aux-toggle");
743758
expect(elements.chooseKeyDiv.style.display).toBe("none");
@@ -1062,7 +1077,7 @@ describe("Toolbar Class", () => {
10621077
test("closeAuxToolbar hides auxiliary toolbar if visible", () => {
10631078
const elements = {
10641079
"aux-toolbar": { style: { display: "block" } },
1065-
"menu": { innerHTML: "" },
1080+
"menu": { innerHTML: "", textContent: "", appendChild: jest.fn() },
10661081
"toggleAuxBtn": {
10671082
className: "some-class blue darken-1",
10681083
classList: {
@@ -1077,7 +1092,7 @@ describe("Toolbar Class", () => {
10771092
toolbar.activity = {};
10781093
toolbar.closeAuxToolbar(mockOnClick);
10791094
expect(elements["aux-toolbar"].style.display).toBe("none");
1080-
expect(elements.menu.innerHTML).toBe("menu");
1095+
expect(elements.menu.textContent).toBe("menu");
10811096
expect(mockOnClick).toHaveBeenCalledWith(toolbar.activity, false);
10821097
expect(elements.toggleAuxBtn.classList.remove).toHaveBeenCalledWith("blue", "darken-1");
10831098
});
@@ -1283,6 +1298,8 @@ describe("Toolbar Class", () => {
12831298
classList: { add: jest.fn(), remove: jest.fn(), contains: jest.fn(() => false) },
12841299
style: { display: "block" },
12851300
innerHTML: "",
1301+
textContent: "",
1302+
appendChild: jest.fn(),
12861303
addEventListener: jest.fn(),
12871304
querySelector: jest.fn(() => ({ textContent: "" })),
12881305
contains: jest.fn(() => false)

‎js/palette.js‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -601,13 +601,13 @@ class Palettes {
601601

602602
if (this.collapsed) {
603603
palette.style.transform = "translateX(-100%)";
604-
document.getElementById("paletteToggle").innerHTML = "▶";
604+
document.getElementById("paletteToggle").textContent = "▶";
605605
document.getElementById("paletteToggle").setAttribute("aria-expanded", "false");
606606
palette.style.transition = "transform 0.3s ease";
607607
this.paletteWidth = 0;
608608
} else {
609609
palette.style.transform = "translateX(0)";
610-
document.getElementById("paletteToggle").innerHTML = "◀";
610+
document.getElementById("paletteToggle").textContent = "◀";
611611
document.getElementById("paletteToggle").setAttribute("aria-expanded", "true");
612612
this.paletteWidth = 55 * PALETTE_WIDTH_FACTOR;
613613
}
@@ -650,7 +650,7 @@ class Palettes {
650650
document.body.appendChild(element);
651651

652652
const toggleBtn = document.createElement("div");
653-
toggleBtn.innerHTML = "◀";
653+
toggleBtn.textContent = "◀";
654654
toggleBtn.id = "paletteToggle";
655655
toggleBtn.setAttribute("role", "button");
656656
toggleBtn.setAttribute("aria-label", _("Toggle Palette"));
@@ -1518,8 +1518,10 @@ class Palette {
15181518
let header = this.menuContainer.children[0];
15191519
header = header.insertRow();
15201520
header.style.backgroundColor = platformColor.paletteLabelBackground;
1521-
header.innerHTML =
1522-
'<td style ="width: 100%; height: 42px; box-sizing: border-box; display: flex; flex-direction: row; align-items: center; justify-content: space-between;"></td>';
1521+
const headerCell = document.createElement("td");
1522+
headerCell.style.cssText =
1523+
"width: 100%; height: 42px; box-sizing: border-box; display: flex; flex-direction: row; align-items: center; justify-content: space-between;";
1524+
header.appendChild(headerCell);
15231525
header = header.children[0];
15241526
header.style.padding = "8px";
15251527

‎js/toolbar-ui.js‎

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ class ToolbarUI {
390390
const elem = docById(obj[0]);
391391
if (strings[i].length === 3) {
392392
if (elem !== undefined && elem !== null) {
393-
elem.innerHTML = obj[1];
393+
elem.textContent = obj[1];
394394
}
395395
} else {
396396
if (elem !== undefined && elem !== null) {
@@ -457,8 +457,12 @@ class ToolbarUI {
457457
renderLogoIcon(onclick) {
458458
const logoIcon = docById("mb-logo");
459459
if (this.language === "ja") {
460-
logoIcon.innerHTML =
461-
'<img style="width: 100%; transform: scale(0.85);" src="images/logo-ja.svg">';
460+
logoIcon.textContent = "";
461+
const logoImg = document.createElement("img");
462+
logoImg.style.width = "100%";
463+
logoImg.style.transform = "scale(0.85)";
464+
logoImg.src = "images/logo-ja.svg";
465+
logoIcon.appendChild(logoImg);
462466
}
463467

464468
logoIcon.onmouseenter = () => {
@@ -763,7 +767,10 @@ class ToolbarUI {
763767

764768
themes.forEach(theme => {
765769
if (safeStorageGet("themePreference") === theme) {
766-
icon.innerHTML = docById(theme).innerHTML;
770+
icon.textContent = "";
771+
Array.from(docById(theme).childNodes).forEach(node =>
772+
icon.appendChild(node.cloneNode(true))
773+
);
767774
}
768775
});
769776

@@ -1042,7 +1049,11 @@ class ToolbarUI {
10421049
Record.classList.remove("hide");
10431050
Record.style.display = "block";
10441051
}
1045-
Record.innerHTML = `<i class="material-icons main">${RECORDBUTTON}</i>`;
1052+
Record.textContent = "";
1053+
const recordIcon = document.createElement("i");
1054+
recordIcon.className = "material-icons main";
1055+
recordIcon.textContent = RECORDBUTTON;
1056+
Record.appendChild(recordIcon);
10461057

10471058
// Remove any existing onclick handler
10481059
Record.onclick = null;
@@ -1059,7 +1070,12 @@ class ToolbarUI {
10591070
RecordDropdownArrow.classList.remove("hide");
10601071
RecordDropdownArrow.style.display = "block";
10611072
}
1062-
RecordDropdownArrow.innerHTML = `<i class="material-icons main" style="font-size: 28px;">arrow_drop_down</i>`;
1073+
RecordDropdownArrow.textContent = "";
1074+
const arrowIcon = document.createElement("i");
1075+
arrowIcon.className = "material-icons main";
1076+
arrowIcon.style.fontSize = "28px";
1077+
arrowIcon.textContent = "arrow_drop_down";
1078+
RecordDropdownArrow.appendChild(arrowIcon);
10631079

10641080
// Create handler function for arrow click
10651081
const arrowClickHandler = () => {
@@ -1191,12 +1207,12 @@ class ToolbarUI {
11911207
if (auxToolbar.style.display === "" || auxToolbar.style.display === "none") {
11921208
onclick(this.activity, false);
11931209
auxToolbar.style.display = "block";
1194-
menuIcon.innerHTML = "more_vert";
1210+
menuIcon.textContent = "more_vert";
11951211
this._setAuxToolbarButtonState(true);
11961212
} else {
11971213
onclick(this.activity, true);
11981214
auxToolbar.style.display = "none";
1199-
menuIcon.innerHTML = "menu";
1215+
menuIcon.textContent = "menu";
12001216
this._setAuxToolbarButtonState(false);
12011217
docById("chooseKeyDiv").style.display = "none";
12021218
docById("movable").style.display = "none";
@@ -2260,7 +2276,7 @@ class ToolbarUI {
22602276
onclick(this.activity, false);
22612277
const menuIcon = docById("menu");
22622278
auxToolbar.style.display = "none";
2263-
menuIcon.innerHTML = "menu";
2279+
menuIcon.textContent = "menu";
22642280
this._setAuxToolbarButtonState(false);
22652281
}
22662282
};

‎js/turtles.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1039,7 +1039,7 @@ Turtles.TurtlesView = class {
10391039
if (auxToolbar.style.display === "block") {
10401040
const menuIcon = docById("menu");
10411041
auxToolbar.style.display = "none";
1042-
menuIcon.innerHTML = "menu";
1042+
menuIcon.textContent = "menu";
10431043
docById("toggleAuxBtn").classList.remove("blue", "darken-1");
10441044
}
10451045
this._expandButton.style.visibility = "visible";
@@ -1062,7 +1062,7 @@ Turtles.TurtlesView = class {
10621062
if (auxToolbar.style.display === "block") {
10631063
const menuIcon = docById("menu");
10641064
auxToolbar.style.display = "none";
1065-
menuIcon.innerHTML = "menu";
1065+
menuIcon.textContent = "menu";
10661066
docById("toggleAuxBtn").classList.remove("blue", "darken-1");
10671067
}
10681068

@@ -1115,7 +1115,7 @@ Turtles.TurtlesView = class {
11151115
if (auxToolbar.style.display === "block") {
11161116
const menuIcon = docById("menu");
11171117
auxToolbar.style.display = "none";
1118-
menuIcon.innerHTML = "menu";
1118+
menuIcon.textContent = "menu";
11191119
docById("toggleAuxBtn").classList.remove("blue", "darken-1");
11201120
}
11211121
this.hideMenu();

‎js/widgets/jseditor.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1070,7 +1070,7 @@ class JSEditor {
10701070
btnDiv.style.lineHeight = "20px";
10711071

10721072
if (lineContent === "") {
1073-
btnDiv.innerHTML = "&nbsp;";
1073+
btnDiv.textContent = "\u00a0";
10741074
} else if (hasDebugger) {
10751075
btnDiv.style.cursor = "pointer";
10761076
btnDiv.style.opacity = "1";
@@ -1255,11 +1255,11 @@ class JSEditor {
12551255
if (this.isOpen) {
12561256
this.isOpen = false;
12571257
editorconsole.style.display = "none";
1258-
if (arrowBtn) arrowBtn.innerHTML = "keyboard_arrow_up";
1258+
if (arrowBtn) arrowBtn.textContent = "keyboard_arrow_up";
12591259
} else {
12601260
this.isOpen = true;
12611261
editorconsole.style.display = "block";
1262-
if (arrowBtn) arrowBtn.innerHTML = "keyboard_arrow_down";
1262+
if (arrowBtn) arrowBtn.textContent = "keyboard_arrow_down";
12631263
}
12641264
}
12651265

0 commit comments

Comments
 (0)