Skip to content

Commit 7e4278c

Browse files
committed
fix(sim-links): require a closed authority before {*} so values cannot move the host
1 parent 3441290 commit 7e4278c

8 files changed

Lines changed: 84 additions & 29 deletions

File tree

‎apps/sim/app/o/[organizationId]/links/sim-link-modal.tsx‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import {
1919
SIM_LINK_VARIABLE,
2020
simLinkFieldsSchema,
2121
} from '@/lib/sim-links/validation'
22-
import { useSaveSimLink } from '@/hooks/queries/sim-links'
22+
import { useCreateSimLink, useUpdateSimLink } from '@/hooks/queries/sim-links'
2323

2424
type Field = 'name' | 'url' | 'defaultUrl' | 'description'
2525

@@ -45,7 +45,9 @@ function draftOf(link: SimLink | undefined, initialName = ''): SimLinkBody {
4545

4646
/** Mount per link (keyed) so each open starts from that link's saved fields. */
4747
export function SimLinkModal({ organizationId, onClose, link, initialName }: SimLinkModalProps) {
48-
const mutation = useSaveSimLink(organizationId)
48+
const create = useCreateSimLink(organizationId)
49+
const update = useUpdateSimLink(organizationId)
50+
const mutation = link ? update : create
4951
const nameTaken = isApiClientError(mutation.error) && mutation.error.status === 409
5052
const [draft, setDraft] = useState(() => draftOf(link, initialName))
5153
const [errors, setErrors] = useState<Partial<Record<Field, string>>>({})
@@ -73,7 +75,8 @@ export function SimLinkModal({ organizationId, onClose, link, initialName }: Sim
7375
)
7476
return
7577
}
76-
mutation.mutate({ linkId: link?.id, body: parsed.data }, { onSuccess: onClose })
78+
if (link) update.mutate({ linkId: link.id, body: parsed.data }, { onSuccess: onClose })
79+
else create.mutate(parsed.data, { onSuccess: onClose })
7780
}
7881

7982
const title = link ? `Edit sim/${link.name}` : 'New Sim link'

‎apps/sim/app/o/[organizationId]/links/sim-links.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ export function SimLinks({ organizationId }: SimLinksProps) {
7575
}
7676
>
7777
<div className={RESOURCE_LIST_STACK}>
78-
{links.error ? (
78+
{links.isLoadingError ? (
7979
<SettingsQueryErrorState
8080
error={links.error}
8181
fallback='Could not load Sim links'

‎apps/sim/app/sim/[[...path]]/page.tsx‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import { serializeSimLinkView } from '@/app/o/[organizationId]/links/search-para
1414

1515
export const metadata: Metadata = {
1616
title: 'Sim link',
17-
referrer: 'no-referrer',
1817
robots: { index: false, follow: false },
1918
}
2019

‎apps/sim/hooks/queries/sim-links.ts‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,20 @@ function useInvalidateSimLinks(organizationId: string) {
4343
return () => queryClient.invalidateQueries({ queryKey: simLinkKeys.lists(organizationId) })
4444
}
4545

46-
/** Creates a link, or replaces the fields of `linkId` when given. */
47-
export function useSaveSimLink(organizationId: string) {
46+
export function useCreateSimLink(organizationId: string) {
4847
const invalidate = useInvalidateSimLinks(organizationId)
4948
return useMutation({
50-
mutationFn: ({ linkId, body }: { linkId?: string; body: SimLinkBody }) =>
51-
linkId
52-
? requestJson(updateSimLinkContract, { params: { id: organizationId, linkId }, body })
53-
: requestJson(createSimLinkContract, { params: { id: organizationId }, body }),
49+
mutationFn: (body: SimLinkBody) =>
50+
requestJson(createSimLinkContract, { params: { id: organizationId }, body }),
51+
onSuccess: invalidate,
52+
})
53+
}
54+
55+
export function useUpdateSimLink(organizationId: string) {
56+
const invalidate = useInvalidateSimLinks(organizationId)
57+
return useMutation({
58+
mutationFn: ({ linkId, body }: { linkId: string; body: SimLinkBody }) =>
59+
requestJson(updateSimLinkContract, { params: { id: organizationId, linkId }, body }),
5460
onSuccess: invalidate,
5561
})
5662
}

‎apps/sim/lib/sim-links/destination.test.ts‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ describe('Sim link destinations', () => {
2727
'https://example.com{*}',
2828
'https://example.com@{*}',
2929
'https://example.com:{*}',
30+
'https://{*}@corp.example.com/',
31+
'https://user:{*}@corp.example.com/',
32+
'https://corp.example.com:{*}/path',
3033
'{*}',
3134
'https:{*}',
3235
'javascript:alert(1)',
@@ -64,6 +67,21 @@ describe('Sim link destinations', () => {
6467
}
6568
)
6669

70+
it.each(['https://{*}@corp.example.com/', 'https://user:{*}@corp.example.com/'])(
71+
'refuses to resolve %s, whose values could move the host',
72+
(url) => {
73+
for (const values of [
74+
['evil.com', 'x'],
75+
['443', 'x'],
76+
['a b', 'x:y'],
77+
]) {
78+
expect(() => resolveSimLinkTarget({ url, defaultUrl: null }, values)).toThrow(
79+
'no fixed http(s) origin'
80+
)
81+
}
82+
}
83+
)
84+
6785
it('fills each variable in order, the last taking every remaining value', () => {
6886
expect(redirectUrl('https://github.com/{*}/issues/{*}', ['sim', 'BUG 12', 'x'])).toBe(
6987
'https://github.com/sim/issues/BUG%2012/x'
@@ -97,8 +115,10 @@ describe('Sim link destinations', () => {
97115
})
98116

99117
it('refuses to redirect to a stored destination that is not http(s)', () => {
100-
expect(() =>
101-
resolveSimLinkTarget({ url: 'javascript:alert(1)', defaultUrl: null }, [])
102-
).toThrow('not an http(s) URL')
118+
for (const url of ['javascript:alert(1)', 'data:text/html,hi']) {
119+
expect(() => resolveSimLinkTarget({ url, defaultUrl: null }, [])).toThrow(
120+
'no fixed http(s) origin'
121+
)
122+
}
103123
})
104124
})
Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
1-
import { isVariableSimLinkUrl, parseHttpUrl, SIM_LINK_VARIABLE } from '@/lib/sim-links/validation'
1+
import {
2+
isVariableSimLinkUrl,
3+
parseHttpUrl,
4+
SIM_LINK_VARIABLE,
5+
simLinkOrigin,
6+
} from '@/lib/sim-links/validation'
27

38
interface SimLinkDestination {
49
url: string
@@ -13,34 +18,45 @@ function encodeSegments(values: readonly string[]): string {
1318
return values.map(encodeURIComponent).join('/')
1419
}
1520

16-
function redirectTo(url: string): SimLinkTarget {
21+
/** Values fill only what follows a closed authority, so leaving `origin` is a broken invariant. */
22+
function redirectWithin(origin: string, url: string): SimLinkTarget {
1723
const parsed = parseHttpUrl(url)
18-
if (!parsed) throw new Error('Sim link destination is not an http(s) URL')
24+
if (parsed?.origin !== origin) throw new Error('Sim link resolved outside its fixed origin')
1925
return { kind: 'redirect', url: parsed.toString() }
2026
}
2127

2228
/**
2329
* Where `sim/<name>/<values…>` leads. A plain link appends the values to its path.
2430
* A variable link fills each `{*}` in order, the last taking every remaining value,
25-
* and opens its default destination when given none.
31+
* and opens its default destination when given none. Every redirect stays on the
32+
* stored destination's origin.
2633
*/
2734
export function resolveSimLinkTarget(
2835
link: SimLinkDestination,
2936
values: readonly string[]
3037
): SimLinkTarget {
38+
const origin = simLinkOrigin(link.url)
39+
if (!origin) throw new Error('Sim link destination has no fixed http(s) origin')
3140
if (!isVariableSimLinkUrl(link.url)) {
32-
if (!values.length) return redirectTo(link.url)
41+
if (!values.length) return redirectWithin(origin, link.url)
3342
const url = new URL(link.url)
3443
url.pathname = `${url.pathname.replace(/\/+$/, '')}/${encodeSegments(values)}`
35-
return redirectTo(url.toString())
44+
return redirectWithin(origin, url.toString())
45+
}
46+
if (!values.length && link.defaultUrl) {
47+
const defaultUrl = parseHttpUrl(link.defaultUrl)
48+
if (!defaultUrl) throw new Error('Sim link default destination is not an http(s) URL')
49+
return { kind: 'redirect', url: defaultUrl.toString() }
3650
}
3751
const parts = link.url.split(SIM_LINK_VARIABLE)
3852
const slots = parts.length - 1
39-
if (!values.length && link.defaultUrl) return redirectTo(link.defaultUrl)
4053
if (values.length < slots) return { kind: 'missing_values', required: slots }
4154
const fills = [
4255
...values.slice(0, slots - 1).map(encodeURIComponent),
4356
encodeSegments(values.slice(slots - 1)),
4457
]
45-
return redirectTo(parts.reduce((url, part, index) => url + fills[index - 1] + part))
58+
return redirectWithin(
59+
origin,
60+
parts.reduce((url, part, index) => url + fills[index - 1] + part)
61+
)
4662
}

‎apps/sim/lib/sim-links/validation.ts‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,17 @@ export function parseHttpUrl(value: string): URL | null {
2424
return url.protocol === 'https:' || url.protocol === 'http:' ? url : null
2525
}
2626

27-
/** Values may fill a destination's path, query, or fragment, never its origin. */
28-
function hasFixedHttpOrigin(url: string): boolean {
29-
const first = parseHttpUrl(url.replaceAll(SIM_LINK_VARIABLE, 'a'))
30-
const second = parseHttpUrl(url.replaceAll(SIM_LINK_VARIABLE, 'b'))
31-
return Boolean(first && second && first.origin === second.origin)
27+
/** A scheme and authority closed by `/`, `?`, or `#`: nothing typed later can extend the host. */
28+
const CLOSED_AUTHORITY = /^https?:\/\/[^/?#]+[/?#]/i
29+
30+
/**
31+
* The origin every visit to `url` must land on, or null when values could change it. Values fill
32+
* only what follows the authority, so the text before the first `{*}` must already close it.
33+
*/
34+
export function simLinkOrigin(url: string): string | null {
35+
const [prefix] = url.split(SIM_LINK_VARIABLE)
36+
if (isVariableSimLinkUrl(url) && !CLOSED_AUTHORITY.test(prefix)) return null
37+
return parseHttpUrl(url.replaceAll(SIM_LINK_VARIABLE, 'value'))?.origin ?? null
3238
}
3339

3440
export const simLinkNameSchema = z
@@ -46,7 +52,7 @@ const destinationSchema = z
4652
.max(URL_MAX_LENGTH, `Use at most ${URL_MAX_LENGTH} characters`)
4753

4854
const simLinkUrlSchema = destinationSchema.refine(
49-
hasFixedHttpOrigin,
55+
(url) => simLinkOrigin(url) !== null,
5056
`Enter a full http:// or https:// URL. ${SIM_LINK_VARIABLE} can't replace the domain.`
5157
)
5258

‎apps/sim/scripts/test-sim-links-e2e.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,12 @@ async function run() {
219219
})
220220

221221
await check('destinations must be http(s) with a fixed origin', async () => {
222-
for (const url of ['javascript:alert(1)', 'https://{*}.example.com', 'example.com']) {
222+
for (const url of [
223+
'javascript:alert(1)',
224+
'https://{*}.example.com',
225+
'https://{*}@corp.example.com/',
226+
'example.com',
227+
]) {
223228
await api(owner, '', { method: 'POST', body: fields({ name: 'bad', url }), expected: 400 })
224229
}
225230
await api(owner, '', {

0 commit comments

Comments
 (0)