From 633d45082ef9c85e156cdb2c3c04b43b3412e296 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Fri, 23 Oct 2026 20:31:28 +0200 Subject: [PATCH] fix(19.7): RB-IN-01 the My account URL check refuses control characters Browsers strip tab and newline from URLs, so "/\t/host" passed the SPA's same-origin check and resolved off-site. The check now refuses any ASCII control character first, as Go's url.Parse does on the server, and an absolute URL must parse with a host, so both sides apply one rule. --- .../account/AccountContributions.vue | 19 ++++++++++++++----- .../account/AccountContributions.test.ts | 15 +++++++++++++++ 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/admin/src/components/account/AccountContributions.vue b/admin/src/components/account/AccountContributions.vue index b01a9362..7d90f579 100644 --- a/admin/src/components/account/AccountContributions.vue +++ b/admin/src/components/account/AccountContributions.vue @@ -23,14 +23,23 @@ const running = ref('') const links = ref>({}) /** - * The URL when it may be rendered and opened: an absolute http(s) URL or a - * same-origin path (one leading "/"), else ''. The server applies the same - * rule; this keeps a javascript: or data: URL out of href and window.open - * even if one gets through. + * The URL when it may be rendered and opened: an absolute http(s) URL with a + * host or a same-origin path (one leading "/"), else ''. The server applies + * the same rule (Go's url.Parse refuses control characters); this keeps a + * javascript: or data: URL out of href and window.open even if one gets + * through. A control character is refused first: browsers strip tab and + * newline, so "/\t/host" would otherwise resolve to //host, off-site. */ function safeUrl(u: string): string { + if (/[\u0000-\u001f\u007f]/.test(u)) { + return '' + } if (/^https?:\/\//i.test(u)) { - return u + try { + return new URL(u).host !== '' ? u : '' + } catch { + return '' + } } if (u.startsWith('/') && !u.startsWith('//') && !u.startsWith('/\\')) { return u diff --git a/admin/tests/account/AccountContributions.test.ts b/admin/tests/account/AccountContributions.test.ts index 3ed4d4cc..1943fbd0 100644 --- a/admin/tests/account/AccountContributions.test.ts +++ b/admin/tests/account/AccountContributions.test.ts @@ -157,6 +157,21 @@ describe('My account contributions', () => { expect(toasts.value.map((toast) => toast.text)).toEqual(['Done.']) }) + // RB-IN-01: browsers strip tab and newline, so these resolve off-site. + for (const url of ['/\t/evil.example/x', '/\n/evil.example', '/\r\n/evil.example', '/x\u0000', 'https://']) { + it(`a ${JSON.stringify(url)} URL answer renders no link and never opens a window`, async () => { + const opened = vi.spyOn(window, 'open').mockImplementation(() => null) + const { wrapper } = await openAccount({ + [`POST ${OPEN}`]: envelope({ message: 'Done.', url }), + }) + await wrapper.find('[data-account-contribution-action="open"]').trigger('click') + await flushPromises() + expect(opened).not.toHaveBeenCalled() + expect(wrapper.find('[data-account-contribution-url]').exists()).toBe(false) + expect(wrapper.html()).not.toContain('evil.example') + }) + } + it('a section with error: true shows the section error text', async () => { setBundle({ ...langFixture.data, ...strings, 'backend::lang.myaccount.section_error': { other: 'This section could not be loaded.' } }, 'en') const { wrapper } = await openAccount({