Skip to content

Commit d80d278

Browse files
Refactor channel name handling in popupDocument to ensure proper JavaScript serialization (#1325)
1 parent ae539ac commit d80d278

2 files changed

Lines changed: 17 additions & 6 deletions

File tree

packages/core/api/src/oauth-popup.test.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,11 @@ describe("popupDocument", () => {
115115
expect(html).not.toContain("<details");
116116
});
117117

118-
it("HTML-escapes the BroadcastChannel name so attacker-controlled names cannot break out", () => {
119-
const html = popupDocument(successPayload, 'evil"name');
120-
expect(html).toContain('new BroadcastChannel("evil&quot;name")');
118+
it("serializes the channel name as JavaScript so the exact channel is preserved", () => {
119+
const html = popupDocument(successPayload, 'evil"name\\path');
120+
expect(html).toContain('new BroadcastChannel("evil\\"name\\\\path")');
121+
expect(html).toContain('localStorage.setItem("evil\\"name\\\\path",JSON.stringify(p))');
122+
expect(html).not.toContain("evil&quot;name");
121123
});
122124

123125
it("escapes < > & in the serialized script payload to prevent </script> breakout", () => {
@@ -138,6 +140,15 @@ describe("popupDocument", () => {
138140
expect(scriptLiteral).toContain("\\u003c/script\\u003e");
139141
});
140142

143+
it("escapes < > & in the serialized channel name to prevent </script> breakout", () => {
144+
const html = popupDocument(successPayload, 'channel</script><img src=x onerror="alert(1)">');
145+
const scriptMatch = /<script>([\s\S]*?)<\/script>/.exec(html);
146+
expect(scriptMatch).not.toBeNull();
147+
const script = scriptMatch![1]!;
148+
expect(script).not.toContain("</script>");
149+
expect(script).toContain("channel\\u003c/script\\u003e");
150+
});
151+
141152
it("posts to window.opener AND falls back to BroadcastChannel with the given channel name", () => {
142153
const html = popupDocument(successPayload, "executor:openapi-oauth-result");
143154
expect(html).toContain("window.opener.postMessage(p,window.location.origin)");

packages/core/api/src/oauth-popup.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ export const popupDocument = <TAuth>(
9191
const icon = payload.ok
9292
? '<path d="M6 10l3 3 5-6" stroke="white" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"/>'
9393
: '<path d="M7 7l6 6M13 7l-6 6" stroke="white" stroke-width="2" stroke-linecap="round"/>';
94-
const escapedChannel = escapeHtml(channelName);
94+
const serializedChannel = serializeForScript(channelName);
9595
const detailsHtml = details
9696
? `<details style="margin-top:16px;text-align:left"><summary style="cursor:pointer;font-size:12px;color:#888;user-select:none">Details</summary><pre style="white-space:pre-wrap;word-break:break-word;font-size:11px;line-height:1.5;color:#52525b;background:#f4f4f5;padding:8px;border-radius:4px;margin:8px 0 0;font-family:ui-monospace,SFMono-Regular,Menlo,monospace">${escapeHtml(details)}</pre></details>`
9797
: "";
@@ -117,8 +117,8 @@ ${detailsHtml}
117117
// raced by the auto-close — so localStorage (a 'storage' event on the opener) is
118118
// the reliable fallback. The opener settles on whichever lands first.
119119
try{if(window.opener)window.opener.postMessage(p,window.location.origin)}catch(e){}
120-
try{if("BroadcastChannel"in window){const c=new BroadcastChannel("${escapedChannel}");c.postMessage(p);setTimeout(()=>c.close(),100)}}catch(e){}
121-
try{localStorage.setItem("${escapedChannel}",JSON.stringify(p))}catch(e){}
120+
try{if("BroadcastChannel"in window){const c=new BroadcastChannel(${serializedChannel});c.postMessage(p);setTimeout(()=>c.close(),100)}}catch(e){}
121+
try{localStorage.setItem(${serializedChannel},JSON.stringify(p))}catch(e){}
122122
if(p.ok)setTimeout(()=>window.close(),400);})();
123123
</script>
124124
</body></html>`;

0 commit comments

Comments
 (0)