Skip to content

Commit 2be92f7

Browse files
authored
Merge pull request #3465 from appfi5/bugfix/release-notes-window
fix: harden release notes rendering and privileged window navigation
2 parents a95688e + e73c9f6 commit 2be92f7

7 files changed

Lines changed: 135 additions & 7 deletions

File tree

packages/neuron-ui/src/components/GeneralSetting/index.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { uniformTimeFormatter, bytesFormatter, clsx, wakeScreen, releaseWakeLock
1212
import Switch from 'widgets/Switch'
1313
import { keepScreenAwake } from 'services/localCache'
1414
import { LanguageSelect, UnLock } from 'widgets/Icons/icon'
15+
import { sanitizeReleaseNotes } from 'utils/sanitizeReleaseNotes'
1516
import styles from './generalSetting.module.scss'
1617
import { useCheckUpdate, useUpdateDownloadStatus } from './hooks'
1718
import LockWindowDialog from './LockWindowDialog'
@@ -67,7 +68,7 @@ const UpdateDownloadStatus = ({
6768

6869
if (available) {
6970
const releaseNotesHtml = () => {
70-
return { __html: releaseNotes }
71+
return { __html: sanitizeReleaseNotes(releaseNotes) }
7172
}
7273

7374
/* eslint-disable react/no-danger */

packages/neuron-ui/src/containers/Navbar/index.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ const Navbar = () => {
111111
}
112112
}, [network?.readonly])
113113

114-
const gotoCompatile = useCallback(() => {
114+
const gotoCompatible = useCallback(() => {
115115
openExternal(`https://neuron.magickbase.com${i18n.language.startsWith('zh') ? '/zh' : ''}/download`)
116116
}, [i18n.language])
117117

@@ -133,7 +133,7 @@ const Navbar = () => {
133133
i18nKey="navbar.ckb-node-compatible"
134134
values={{ version: getVersion(), btnText: t('navbar.learn-more') }}
135135
components={[
136-
<button type="button" className={styles.learnMore} onClick={gotoCompatile}>
136+
<button type="button" className={styles.learnMore} onClick={gotoCompatible}>
137137
{t('navbar.learn-more')}
138138
</button>,
139139
]}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { describe, expect, it } from 'vitest'
2+
import { sanitizeReleaseNotes } from 'utils/sanitizeReleaseNotes'
3+
4+
describe('sanitizeReleaseNotes', () => {
5+
it('removes interactive markup and keeps basic formatting', () => {
6+
const sanitized = sanitizeReleaseNotes(`
7+
<p>safe</p>
8+
<button data-method="open-in-window" onclick="window.electron.ipcRenderer.invoke('open-in-window')">open</button>
9+
<img src="x" onerror="alert('xss')" />
10+
<script>alert('xss')</script>
11+
<pre><code>const ok = true</code></pre>
12+
`)
13+
14+
expect(sanitized).toContain('<p>safe</p>')
15+
expect(sanitized).toContain('<pre><code>const ok = true</code></pre>')
16+
expect(sanitized).not.toContain('<button')
17+
expect(sanitized).not.toContain('<img')
18+
expect(sanitized).not.toContain('<script')
19+
expect(sanitized).not.toContain('onclick')
20+
expect(sanitized).not.toContain('data-method')
21+
expect(sanitized).not.toContain('onerror')
22+
expect(sanitized).not.toContain("alert('xss')")
23+
})
24+
})
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
const ALLOWED_TAGS = new Set(['p', 'br', 'ul', 'ol', 'li', 'strong', 'em', 'b', 'i', 'code', 'pre'])
2+
const DROP_CONTENT_TAGS = new Set(['script', 'style', 'iframe', 'object', 'embed'])
3+
4+
const sanitizeNodes = (doc: Document, nodes: ChildNode[]): Node[] => {
5+
return nodes.flatMap(node => {
6+
if (node.nodeType === Node.TEXT_NODE) {
7+
return [doc.createTextNode(node.textContent ?? '')]
8+
}
9+
10+
if (node.nodeType !== Node.ELEMENT_NODE) {
11+
return []
12+
}
13+
14+
const element = node as HTMLElement
15+
const tagName = element.tagName.toLowerCase()
16+
17+
if (DROP_CONTENT_TAGS.has(tagName)) {
18+
return []
19+
}
20+
21+
const children = sanitizeNodes(doc, Array.from(element.childNodes))
22+
23+
if (!ALLOWED_TAGS.has(tagName)) {
24+
return children
25+
}
26+
27+
const sanitizedElement = doc.createElement(tagName)
28+
children.forEach(child => {
29+
sanitizedElement.appendChild(child)
30+
})
31+
return [sanitizedElement]
32+
})
33+
}
34+
35+
export const sanitizeReleaseNotes = (releaseNotes: string) => {
36+
const template = document.createElement('template')
37+
template.innerHTML = releaseNotes
38+
39+
const container = document.createElement('div')
40+
sanitizeNodes(document, Array.from(template.content.childNodes)).forEach(node => {
41+
container.appendChild(node)
42+
})
43+
return container.innerHTML
44+
}
45+
46+
export default sanitizeReleaseNotes
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import env from '../../env'
2+
3+
export const resolveInternalWindowTarget = (url: string) => {
4+
const normalizedUrl = url.trim()
5+
6+
if (normalizedUrl.startsWith('#/')) {
7+
return {
8+
navigationUrl: normalizedUrl.replace(/^#/, ''),
9+
windowUrl: `${env.mainURL}${normalizedUrl}`,
10+
}
11+
}
12+
13+
if (normalizedUrl.startsWith('/')) {
14+
return {
15+
navigationUrl: normalizedUrl,
16+
windowUrl: `${env.mainURL}#${normalizedUrl}`,
17+
}
18+
}
19+
20+
return null
21+
}

packages/neuron-wallet/src/controllers/app/show-window.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,23 @@ import { BrowserWindow } from 'electron'
22
import path from 'path'
33
import env from '../../env'
44
import AppController from '.'
5+
import { resolveInternalWindowTarget } from './resolve-window-url'
56

67
const showWindow = (
78
url: string,
89
title: string,
910
options?: Electron.BrowserWindowConstructorOptions,
1011
channels?: string[],
1112
comparator: (win: BrowserWindow) => boolean = win => win.getTitle() === title
12-
): BrowserWindow => {
13+
): BrowserWindow | null => {
14+
const target = resolveInternalWindowTarget(url)
15+
if (!target) {
16+
return null
17+
}
18+
1319
const opened = BrowserWindow.getAllWindows().find(comparator)
1420
if (opened) {
15-
opened.webContents.send('navigation', url.replace(/^#/, ''))
21+
opened.webContents.send('navigation', target.navigationUrl)
1622
opened.focus()
1723
return opened
1824
} else {
@@ -38,8 +44,7 @@ const showWindow = (
3844
if (channels) {
3945
AppController.getInstance().registerChannels(win, channels)
4046
}
41-
const fmtUrl = url.startsWith('http') || url.startsWith('file:') ? url : env.mainURL + url
42-
win.loadURL(fmtUrl)
47+
win.loadURL(target.windowUrl)
4348
win.on('ready-to-show', () => {
4449
win.setTitle(title)
4550
win.show()
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
jest.mock('../../../src/env', () => ({
2+
__esModule: true,
3+
default: {
4+
mainURL: 'file:///app/index.html',
5+
},
6+
}))
7+
8+
import { resolveInternalWindowTarget } from '../../../src/controllers/app/resolve-window-url'
9+
10+
describe('resolveInternalWindowTarget', () => {
11+
it('accepts internal hash routes', () => {
12+
expect(resolveInternalWindowTarget(' #/settings/general ')).toEqual({
13+
navigationUrl: '/settings/general',
14+
windowUrl: 'file:///app/index.html#/settings/general',
15+
})
16+
})
17+
18+
it('accepts internal slash routes', () => {
19+
expect(resolveInternalWindowTarget('/settings/general')).toEqual({
20+
navigationUrl: '/settings/general',
21+
windowUrl: 'file:///app/index.html#/settings/general',
22+
})
23+
})
24+
25+
it.each(['https://attacker.example/poc', 'http://127.0.0.1:3000/#/settings', 'file:///tmp/poc.html'])(
26+
'rejects external target %s',
27+
target => {
28+
expect(resolveInternalWindowTarget(target)).toBeNull()
29+
}
30+
)
31+
})

0 commit comments

Comments
 (0)