fix(sim-links): harden destinations against dot segments, control characters, and off-site defaults

Author: waleedlatif1

apps/docs/content/docs/search/sim-links.mdx

@@ -49,7 +49,7 @@ To place a value somewhere else, put `{*}` in the destination URL:
 | `https://wiki.example.com/search?q={*}` | `sim/wiki/expense policy` | `https://wiki.example.com/search?q=expense%20policy` |
 | `https://github.com/acme/{*}/issues/{*}` | `sim/issues/api/42` | `https://github.com/acme/api/issues/42` |
 
-Each value fills the next `{*}` in order, and the last `{*}` takes everything that remains. A link with `{*}` can also have a **Default destination**, which opens when you use the link without a value. `{*}` can't replace the domain.
+Each value fills the next `{*}` in order, and the last `{*}` takes everything that remains. A link with `{*}` can also have a **Default destination** on the same site, which opens when you use the link without a value. `{*}` can't replace the domain, and values can't be `.` or `..`.
 
 ## Control access
 

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

@@ -5,7 +5,7 @@ import { getSession } from '@/lib/auth'
 import { getActiveOrganizationId } from '@/lib/auth/session-response'
 import { organizationRoutes, simLinkPath } from '@/lib/navigation/paths'
 import { resolveOrganizationLanding } from '@/lib/organizations/surface'
-import { resolveSimLink } from '@/lib/sim-links/application/use-cases'
+import { resolveSimLink, type SimLinkResolution } from '@/lib/sim-links/application/use-cases'
 import { isSimLinksEnabled } from '@/lib/sim-links/feature-flag'
 import { simLinkNameSchema } from '@/lib/sim-links/validation'
 import { buildAuthCrossLink } from '@/app/(auth)/auth-redirect'
@@ -32,6 +32,20 @@ function decodeSimLinkPath(path: string[]): string[] {
   return segments.length === 1 ? segments[0].split('/').filter(Boolean) : segments
 }
 
+/**
+ * Why a link did not redirect. Only a valid link name is echoed back, so an arbitrary path
+ * can't put its own text on this page.
+ */
+function describeUnresolved(result: SimLinkResolution, validName: boolean, name: string): string {
+  if (result.kind === 'missing_values') {
+    const example = Array.from({ length: result.required }, () => 'value').join('/')
+    const needed = result.required === 1 ? 'a value' : `${result.required} values`
+    return `This link needs ${needed}, like sim/${name}/${example}.`
+  }
+  if (result.kind === 'invalid_values') return "Sim links can't open . or .. as a value."
+  return validName ? "This Sim link doesn't exist yet." : "That isn't a valid Sim link name."
+}
+
 /**
  * Where `sim/<name>/<values…>` lands: the viewer's organization link's destination,
  * or a page that offers to create the link when nobody has yet.
@@ -62,16 +76,13 @@ export default async function SimLinkPage({ params }: SimLinkPageProps) {
   })
   if (result.kind === 'redirect') redirect(result.url)
 
-  const creatable = result.kind === 'not_found' && simLinkNameSchema.safeParse(name).success
+  const validName = simLinkNameSchema.safeParse(name).success
+  const creatable = result.kind === 'not_found' && validName
   return (
     <LogoShell center theme='inherit'>
       <StatusPageContent
-        title={`sim/${name}`}
-        description={
-          result.kind === 'missing_values'
-            ? `This link needs ${result.required === 1 ? 'a value' : `${result.required} values`}, like sim/${name}/${Array.from({ length: result.required }, () => 'value').join('/')}.`
-            : "This Sim link doesn't exist yet."
-        }
+        title={validName ? `sim/${name}` : 'Sim link'}
+        description={describeUnresolved(result, validName, name)}
       >
         {creatable && (
           <ChipLink variant='primary' href={serializeSimLinkView(library, { create: name })}>

apps/sim/lib/sim-links/application/use-cases.ts

@@ -32,7 +32,7 @@ export interface SimLinkView extends SimLinkRecord {
   canManage: boolean
 }
 
-type SimLinkResolution = SimLinkTarget | { kind: 'not_found' }
+export type SimLinkResolution = SimLinkTarget | { kind: 'not_found' }
 
 const rollout = {
   authorizeResource: ({ context }: { context: OrganizationMembershipContext }) =>
@@ -112,7 +112,12 @@ function auditOf(
 
 /** What a saved link now does, for its audit entry. */
 function savedMetadata(link: SimLinkView): Record<string, unknown> {
-  return { url: link.url, unlisted: link.unlisted, restrictEdit: link.restrictEdit }
+  return {
+    url: link.url,
+    defaultUrl: link.defaultUrl,
+    unlisted: link.unlisted,
+    restrictEdit: link.restrictEdit,
+  }
 }
 
 /** Read in the writing transaction, so the result and its audit describe exactly this write. */

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

@@ -35,6 +35,9 @@ describe('Sim link destinations', () => {
     'ftp://example.com/file',
     '//example.com/path',
     'example.com',
+    'https://\t/{*}',
+    'https://\n/{*}',
+    'https://example.com/\r{*}',
   ])('rejects %s, whose origin is not a fixed http(s) origin', (url) => {
     expect(acceptsDestination(url)).toBe(false)
   })
@@ -53,6 +56,9 @@ describe('Sim link destinations', () => {
     expect(acceptsDestination('https://example.com', 'https://example.com/other')).toBe(false)
     expect(acceptsDestination('https://example.com/{*}', 'https://example.com/{*}')).toBe(false)
     expect(acceptsDestination('https://example.com/{*}', 'javascript:alert(1)')).toBe(false)
+    expect(acceptsDestination('https://example.com/{*}', 'https://elsewhere.example.org')).toBe(
+      false
+    )
   })
 
   it.each(['//evil.com', '@evil.com', '../../admin', '?x=1#y', '%2Fetc', 'a:b'])(
@@ -74,6 +80,17 @@ describe('Sim link destinations', () => {
     }
   )
 
+  it.each([[['..', '..', 'attacker', 'phish']], [['.']], [['a', '..']]])(
+    'refuses dot segments in %j, which would climb out of the link path',
+    (values) => {
+      for (const url of ['https://github.com/acme/{*}', 'https://github.com/acme/handbook']) {
+        expect(resolveSimLinkTarget({ url, defaultUrl: null }, values)).toEqual({
+          kind: 'invalid_values',
+        })
+      }
+    }
+  )
+
   it('fills each variable in order, the last taking every remaining value', () => {
     expect(redirectUrl('https://github.com/{*}/issues/{*}', ['sim', 'BUG 12', 'x'])).toBe(
       'https://github.com/sim/issues/BUG%2012/x'

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

@@ -13,6 +13,12 @@ interface SimLinkDestination {
 export type SimLinkTarget =
   | { kind: 'redirect'; url: string }
   | { kind: 'missing_values'; required: number }
+  | { kind: 'invalid_values' }
+
+/** URL parsers resolve `.` and `..` even when encoded, so a value could climb out of the link's path. */
+function isDotSegment(value: string): boolean {
+  return value === '.' || value === '..'
+}
 
 function encodeSegments(values: readonly string[]): string {
   return values.map(encodeURIComponent).join('/')
@@ -37,17 +43,14 @@ export function resolveSimLinkTarget(
 ): SimLinkTarget {
   const origin = simLinkOrigin(link.url)
   if (!origin) throw new Error('Sim link destination has no fixed http(s) origin')
+  if (values.some(isDotSegment)) return { kind: 'invalid_values' }
   if (!isVariableSimLinkUrl(link.url)) {
     if (!values.length) return redirectWithin(origin, link.url)
     const url = new URL(link.url)
     url.pathname = `${url.pathname.replace(/\/+$/, '')}/${encodeSegments(values)}`
     return redirectWithin(origin, url.toString())
   }
-  if (!values.length && link.defaultUrl) {
-    const defaultUrl = parseHttpUrl(link.defaultUrl)
-    if (!defaultUrl) throw new Error('Sim link default destination is not an http(s) URL')
-    return { kind: 'redirect', url: defaultUrl.toString() }
-  }
+  if (!values.length && link.defaultUrl) return redirectWithin(origin, link.defaultUrl)
   const parts = link.url.split(SIM_LINK_VARIABLE)
   const slots = parts.length - 1
   if (values.length < slots) return { kind: 'missing_values', required: slots }

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

@@ -31,11 +31,15 @@ export function parseHttpUrl(value: string): URL | null {
 /** A scheme and authority closed by `/`, `?`, or `#`: nothing typed later can extend the host. */
 const CLOSED_AUTHORITY = /^https?:\/\/[^/?#]+[/?#]/i
 
+/** URL parsers strip tabs and line breaks, so one inside the authority could hide where it ends. */
+const CONTROL_CHARACTER = /[\u0000-\u001f\u007f]/
+
 /**
  * The origin every visit to `url` must land on, or null when values could change it. Values fill
  * only what follows the authority, so the text before the first `{*}` must already close it.
  */
 export function simLinkOrigin(url: string): string | null {
+  if (CONTROL_CHARACTER.test(url)) return null
   const [prefix] = url.split(SIM_LINK_VARIABLE)
   if (isVariableSimLinkUrl(url) && !CLOSED_AUTHORITY.test(prefix)) return null
   return parseHttpUrl(url.replaceAll(SIM_LINK_VARIABLE, 'value'))?.origin ?? null
@@ -61,7 +65,7 @@ const simLinkUrlSchema = destinationSchema.refine(
 )
 
 const defaultUrlSchema = destinationSchema.refine(
-  (url) => !isVariableSimLinkUrl(url) && parseHttpUrl(url) !== null,
+  (url) => !isVariableSimLinkUrl(url) && simLinkOrigin(url) !== null,
   'Enter a full http:// or https:// URL'
 )
 
@@ -88,5 +92,10 @@ export const simLinkFieldsSchema = z
     path: ['defaultUrl'],
     message: `Only a link with ${SIM_LINK_VARIABLE} has a default destination`,
   })
+  .refine(
+    (fields) =>
+      !fields.defaultUrl || simLinkOrigin(fields.defaultUrl) === simLinkOrigin(fields.url),
+    { path: ['defaultUrl'], message: 'Use a page on the same site as the destination URL' }
+  )
 
 export type SimLinkFields = z.output<typeof simLinkFieldsSchema>

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

@@ -233,13 +233,16 @@ async function run() {
     )
     await create(owner, { name: 'gh', url: 'https://github.com/{*}/issues/{*}' })
     assert.match((await open(member, 'gh/sim')).text, /needs 2 values/)
+    assert.match((await open(member, 'jira%2F..%2F..%2Fadmin')).text, /can.*t open/)
+    assert.doesNotMatch((await open(member, 'not%20a%20link%20name')).text, /not a link name/)
   })
 
   await check('destinations must be http(s) with a fixed origin', async () => {
     for (const url of [
       'javascript:alert(1)',
       'https://{*}.example.com',
       'https://{*}@corp.example.com/',
+      'https://\t/{*}',
       'example.com',
     ]) {
       await api(owner, '', { method: 'POST', body: fields({ name: 'bad', url }), expected: 400 })
@@ -253,6 +256,15 @@ async function run() {
       }),
       expected: 400,
     })
+    await api(owner, '', {
+      method: 'POST',
+      body: fields({
+        name: 'bad',
+        url: 'https://example.com/{*}',
+        defaultUrl: 'https://elsewhere.example.org',
+      }),
+      expected: 400,
+    })
   })
 
   await check('names differing only in case or punctuation conflict', async () => {