Fix SSRF in Link Preview Service (#10689)

Signed-off-by: Artem Savchenko <armisav@gmail.com>
2026-09-30 13:35:04 +02:00 · 2026-03-27 11:06:59 +07:00
parent 2b6d065217
commit c2cf3a85cc
2 changed files with 330 additions and 39 deletions
@@ -89,6 +89,18 @@ describe('URL Validation', () => {
    it('should reject invalid URLs', async () => {
      await expect(parseLinkPreviewDetails(ctx, defaultConfig, 'not-a-valid-url')).rejects.toThrow(LinkPreviewError)
    })
+
+    it('should set INVALID_URL code for malformed URLs', async () => {
+      await expect(parseLinkPreviewDetails(ctx, defaultConfig, 'not-a-valid-url')).rejects.toMatchObject({
+        code: 'INVALID_URL'
+      })
+    })
+
+    it('should set INVALID_PROTOCOL code for non-http protocols', async () => {
+      await expect(parseLinkPreviewDetails(ctx, defaultConfig, 'javascript:alert(1)')).rejects.toMatchObject({
+        code: 'INVALID_PROTOCOL'
+      })
+    })
  })

  describe('SSRF protection', () => {
@@ -103,7 +115,13 @@ describe('URL Validation', () => {
      '192.168.255.255',
      '169.254.0.1',
      '0.0.0.0',
-      'localhost'
+      'localhost',
+      'localhost.',
+      '[::1]',
+      // IPv6-mapped IPv4 loopback (hex form used in the report)
+      '[::ffff:7f00:1]',
+      // IPv6-mapped IPv4 loopback (dotted form)
+      '[::ffff:127.0.0.1]'
    ]

    it.each(blockedAddresses)('should block access to %s', async (host) => {
@@ -120,6 +138,24 @@ describe('URL Validation', () => {
      const result = await parseLinkPreviewDetails(ctx, defaultConfig, 'https://8.8.8.8')
      expect(result).toBeDefined()
    })
+
+    it('should block unique-local and link-local IPv6 addresses', async () => {
+      await expect(parseLinkPreviewDetails(ctx, defaultConfig, 'https://[fc00::1]/path')).rejects.toThrow(
+        LinkPreviewError
+      )
+      await expect(parseLinkPreviewDetails(ctx, defaultConfig, 'https://[fd12::abcd]/path')).rejects.toThrow(
+        LinkPreviewError
+      )
+      await expect(parseLinkPreviewDetails(ctx, defaultConfig, 'https://[fe80::1]/path')).rejects.toThrow(
+        LinkPreviewError
+      )
+    })
+
+    it('should set BLOCKED_URL code for blocked hosts', async () => {
+      await expect(parseLinkPreviewDetails(ctx, defaultConfig, 'https://localhost./path')).rejects.toMatchObject({
+        code: 'BLOCKED_URL'
+      })
+    })
  })
 })

@@ -469,6 +505,50 @@ describe('oEmbed Integration', () => {
    expect(result.imageWidth).toBe(1920)
    expect(result.imageHeight).toBe(1080)
  })
+
+  it('should block SSRF via oEmbed discovery URL (internal destination)', async () => {
+    const html = `
+      <html>
+        <head>
+          <link type="application/json+oembed" href="http://[::ffff:7f00:1]:7777/oembed-ssrf">
+          <meta property="og:title" content="OG Title">
+        </head>
+      </html>
+    `
+
+    global.fetch = jest.fn().mockImplementation((url: string, init?: RequestInit) => {
+      // Implementation should not rely on automatic redirect following.
+      if (init?.redirect === 'follow') return Promise.reject(new Error('redirect: follow should not be used'))
+
+      if (url === 'https://example.com/attacker') return Promise.resolve(createHtmlResponse(html))
+      // oEmbed fetch must not happen (blocked by URL validation).
+      return Promise.reject(new Error(`Unexpected fetch to ${url}`))
+    })
+
+    await expect(parseLinkPreviewDetails(ctx, defaultConfig, 'https://example.com/attacker')).rejects.toThrow(
+      LinkPreviewError
+    )
+  })
+
+  it('should fall back to OG when oEmbed request throws network error', async () => {
+    const html = `
+      <html>
+        <head>
+          <link type="application/json+oembed" href="https://example.com/oembed">
+          <meta property="og:title" content="OG Fallback Title">
+        </head>
+      </html>
+    `
+
+    global.fetch = jest.fn().mockImplementation((url: string) => {
+      if (url === 'https://example.com/oembed') return Promise.reject(new Error('oEmbed network down'))
+      if (url === 'https://example.com/page') return Promise.resolve(createHtmlResponse(html))
+      return Promise.reject(new Error(`Unexpected fetch to ${url}`))
+    })
+
+    const result = await parseLinkPreviewDetails(ctx, defaultConfig, 'https://example.com/page')
+    expect(result.title).toBe('OG Fallback Title')
+  })
 })

 // ============================================================================
@@ -568,6 +648,14 @@ describe('Error Handling', () => {
    await expect(parseLinkPreviewDetails(ctx, defaultConfig, 'https://example.com')).rejects.toThrow(LinkPreviewError)
  })

+  it('should set FETCH_FAILED code on network errors', async () => {
+    global.fetch = jest.fn().mockRejectedValue(new Error('Network error'))
+
+    await expect(parseLinkPreviewDetails(ctx, defaultConfig, 'https://example.com')).rejects.toMatchObject({
+      code: 'FETCH_FAILED'
+    })
+  })
+
  it('should handle malformed HTML gracefully', async () => {
    const html = '<html><head><title>Broken<<<<</title></head></html>'
    global.fetch = jest.fn().mockResolvedValue(createHtmlResponse(html))
@@ -726,6 +814,72 @@ describe('Edge Cases', () => {
    expect(result.title).toBe('Redirected')
  })

+  it('should block SSRF via redirects to internal addresses', async () => {
+    global.fetch = jest.fn().mockImplementation((url: string, init?: RequestInit) => {
+      if (init?.redirect === 'follow') return Promise.reject(new Error('redirect: follow should not be used'))
+
+      if (url === 'https://example.com/start') {
+        return Promise.resolve(
+          new Response(null, {
+            status: 302,
+            headers: { location: 'http://[::ffff:7f00:1]:9999/' }
+          })
+        )
+      }
+
+      // Redirect target should never be fetched if validation is correct.
+      return Promise.reject(new Error(`Unexpected fetch to ${url}`))
+    })
+
+    await expect(parseLinkPreviewDetails(ctx, defaultConfig, 'https://example.com/start')).rejects.toThrow(
+      LinkPreviewError
+    )
+  })
+
+  it('should keep current URL when redirect response has no location', async () => {
+    const html = '<html><head><title>No Location Redirect</title></head></html>'
+    global.fetch = jest.fn().mockImplementation((url: string) => {
+      if (url === 'https://example.com/start') return Promise.resolve(new Response(null, { status: 302 }))
+      return Promise.resolve(createHtmlResponse(html))
+    })
+
+    await expect(parseLinkPreviewDetails(ctx, defaultConfig, 'https://example.com/start')).rejects.toThrow(
+      LinkPreviewError
+    )
+  })
+
+  it('should resolve relative redirect locations', async () => {
+    const html = '<html><head><title>Relative Redirect</title></head></html>'
+    global.fetch = jest.fn().mockImplementation((url: string) => {
+      if (url === 'https://example.com/start') {
+        return Promise.resolve(
+          new Response(null, {
+            status: 302,
+            headers: { location: '/next' }
+          })
+        )
+      }
+      if (url === 'https://example.com/next') return Promise.resolve(createHtmlResponse(html))
+      return Promise.reject(new Error(`Unexpected fetch to ${url}`))
+    })
+
+    const result = await parseLinkPreviewDetails(ctx, defaultConfig, 'https://example.com/start')
+    expect(result.title).toBe('Relative Redirect')
+  })
+
+  it('should fail after too many redirects', async () => {
+    global.fetch = jest.fn().mockResolvedValue(
+      new Response(null, {
+        status: 302,
+        headers: { location: '/loop' }
+      })
+    )
+
+    await expect(parseLinkPreviewDetails(ctx, defaultConfig, 'https://example.com/start')).rejects.toThrow(
+      'Too many redirects'
+    )
+  })
+
  it('should sanitize HTML entities in titles', async () => {
    const html = '<html><head><title>Rock &amp; Roll</title></head></html>'
    global.fetch = jest.fn().mockResolvedValue(createHtmlResponse(html))
@@ -17,6 +17,7 @@ import { MeasureContext } from '@hcengineering/core'
 import * as cheerio from 'cheerio'
 import { imageSize } from 'image-size'
 import oembedProviders from 'oembed-providers'
+import net from 'node:net'

 // ============================================================================
 // Types and Interfaces
@@ -89,20 +90,6 @@ const DEFAULT_TIMEOUT_MS = 10_000
 const DEFAULT_MAX_IMAGE_BYTES = 10 * 1024 * 1024 // 10MB
 const OEMBED_SERVICE_NAME = 'Huly Link Preview Service/1.0'

-// Private IP ranges to block for SSRF protection
-const BLOCKED_IP_PATTERNS = [
-  /^127\./, // Loopback
-  /^10\./, // Private Class A
-  /^172\.(1[6-9]|2\d|3[01])\./, // Private Class B
-  /^192\.168\./, // Private Class C
-  /^169\.254\./, // Link-local
-  /^0\./, // Current network
-  /^localhost$/i,
-  /^::1$/, // IPv6 loopback
-  /^fc00:/i, // IPv6 private
-  /^fe80:/i // IPv6 link-local
-]
-
 // ============================================================================
 // Error Classes
 // ============================================================================
@@ -110,6 +97,7 @@ const BLOCKED_IP_PATTERNS = [
 export class LinkPreviewError extends Error {
  constructor (
    message: string,
+    public readonly code?: 'BLOCKED_URL' | 'INVALID_URL' | 'INVALID_PROTOCOL' | 'TIMEOUT' | 'FETCH_FAILED',
    public readonly cause?: unknown
  ) {
    super(message)
@@ -121,30 +109,165 @@ export class LinkPreviewError extends Error {
 // URL Validation
 // ============================================================================

+function normalizeHostnameForChecks (hostname: string): string {
+  // URL.hostname is already punycode-normalized by WHATWG URL for IDNs.
+  // Keep it lowercase for comparisons.
+  const trimmed = hostname.trim().toLowerCase().replace(/\.+$/, '')
+  if (trimmed.startsWith('[') && trimmed.endsWith(']')) return trimmed.slice(1, -1)
+  return trimmed
+}
+
+function parseIpv6MappedIpv4 (ipv6: string): string | undefined {
+  const host = ipv6.toLowerCase()
+
+  // Common form: ::ffff:127.0.0.1
+  const dotted = host.match(/(?:^|:)ffff:(\d{1,3}(?:\.\d{1,3}){3})$/)
+  if (dotted?.[1] !== undefined) return dotted[1]
+
+  // Hex form used in the report: ::ffff:7f00:1  (=> 127.0.0.1)
+  const hex = host.match(/(?:^|:)ffff:([0-9a-f]{1,4}):([0-9a-f]{1,4})$/)
+  if (hex?.[1] === undefined || hex?.[2] === undefined) return undefined
+
+  const hi = Number.parseInt(hex[1], 16)
+  const lo = Number.parseInt(hex[2], 16)
+  if (!Number.isFinite(hi) || !Number.isFinite(lo)) return undefined
+
+  const a = (hi >> 8) & 0xff
+  const b = hi & 0xff
+  const c = (lo >> 8) & 0xff
+  const d = lo & 0xff
+  return `${a}.${b}.${c}.${d}`
+}
+
+function isBlockedIpv4 (ipv4: string): boolean {
+  const parts = ipv4.split('.').map((p) => Number.parseInt(p, 10))
+  if (parts.length !== 4 || parts.some((n) => !Number.isFinite(n) || n < 0 || n > 255)) return true
+
+  const [a, b] = parts
+
+  // 0.0.0.0/8
+  if (a === 0) return true
+  // 127.0.0.0/8 loopback
+  if (a === 127) return true
+  // 10.0.0.0/8
+  if (a === 10) return true
+  // 172.16.0.0/12
+  if (a === 172 && b >= 16 && b <= 31) return true
+  // 192.168.0.0/16
+  if (a === 192 && b === 168) return true
+  // 169.254.0.0/16 link-local
+  if (a === 169 && b === 254) return true
+
+  return false
+}
+
+function isBlockedIpv6 (ipv6: string): boolean {
+  const host = ipv6.toLowerCase()
+  // unspecified / loopback (compressed or expanded)
+  if (host === '::' || host === '0:0:0:0:0:0:0:0') return true
+  if (host === '::1' || host === '0:0:0:0:0:0:0:1') return true
+  if (host.startsWith('fc') || host.startsWith('fd')) return true // unique-local fc00::/7 (coarse but safe)
+  if (
+    host.startsWith('fe80:') ||
+    host.startsWith('fe8') ||
+    host.startsWith('fe9') ||
+    host.startsWith('fea') ||
+    host.startsWith('feb')
+  ) {
+    // link-local fe80::/10 (coarse but safe)
+    return true
+  }
+
+  const mapped = parseIpv6MappedIpv4(host)
+  if (mapped !== undefined) return isBlockedIpv4(mapped)
+
+  return false
+}
+
+function isBlockedHost (hostname: string): boolean {
+  const host = normalizeHostnameForChecks(hostname)
+  if (host === 'localhost') return true
+
+  const ipType = net.isIP(host)
+  if (ipType === 4) return isBlockedIpv4(host)
+  if (ipType === 6) return isBlockedIpv6(host)
+
+  // Some Node versions are stricter about IPv6 parsing. If it still looks like an IPv6 literal,
+  // apply our IPv6 checks anyway (covers IPv6-mapped IPv4 forms like ::ffff:7f00:1).
+  if (host.includes(':') && isBlockedIpv6(host)) return true
+
+  // Hostname is not an IP literal. Keep legacy explicit localhost-ish blocks.
+  // (We intentionally do not attempt DNS resolution here.)
+  if (host.endsWith('.localhost')) return true
+
+  return false
+}
+
 function validateUrl (urlString: string): URL {
  let url: URL
  try {
    url = new URL(urlString)
  } catch {
-    throw new LinkPreviewError(`Invalid URL: ${urlString}`)
+    throw new LinkPreviewError(`Invalid URL: ${urlString}`, 'INVALID_URL')
  }

  // Only allow HTTP(S) protocols
  if (!['http:', 'https:'].includes(url.protocol)) {
-    throw new LinkPreviewError(`Invalid protocol: ${url.protocol}. Only HTTP and HTTPS are allowed.`)
+    throw new LinkPreviewError(
+      `Invalid protocol: ${url.protocol}. Only HTTP and HTTPS are allowed.`,
+      'INVALID_PROTOCOL'
+    )
  }

-  // SSRF protection: block private/internal IPs
-  const hostname = url.hostname
-  for (const pattern of BLOCKED_IP_PATTERNS) {
-    if (pattern.test(hostname)) {
-      throw new LinkPreviewError('Blocked URL: Access to internal addresses is not allowed.')
-    }
+  // SSRF protection: block private/internal hosts and IP literals (incl. IPv6-mapped IPv4)
+  if (isBlockedHost(url.hostname)) {
+    throw new LinkPreviewError('Blocked URL: Access to internal addresses is not allowed.', 'BLOCKED_URL')
  }

  return url
 }

+function isRedirectStatus (status: number): boolean {
+  return status >= 300 && status < 400
+}
+
+async function fetchWithValidatedRedirects (
+  url: string,
+  options: RequestInit,
+  timeoutMs: number,
+  maxRedirects: number = 5
+): Promise<{ response: Response, finalUrl: string }> {
+  let currentUrl = url
+
+  for (let i = 0; i <= maxRedirects; i++) {
+    // Validate every hop (including the initial request URL).
+    validateUrl(currentUrl)
+
+    const response = await fetchWithTimeout(
+      currentUrl,
+      {
+        ...options,
+        redirect: 'manual'
+      },
+      timeoutMs
+    )
+
+    if (!isRedirectStatus(response.status)) {
+      return { response, finalUrl: currentUrl }
+    }
+
+    const location = response.headers.get('location')
+    if (!isNonEmptyString(location)) {
+      return { response, finalUrl: currentUrl }
+    }
+
+    const nextUrl = new URL(location, currentUrl).href
+    currentUrl = nextUrl
+  }
+
+  throw new LinkPreviewError('Too many redirects')
+}
+
 // ============================================================================
 // Fetch Utilities
 // ============================================================================
@@ -163,10 +286,11 @@ async function fetchWithTimeout (url: string, options: RequestInit, timeoutMs: n
    return response
  } catch (error) {
    if (error instanceof Error && error.name === 'AbortError') {
-      throw new LinkPreviewError(`Request timed out after ${timeoutMs}ms`, error)
+      throw new LinkPreviewError(`Request timed out after ${timeoutMs}ms`, 'TIMEOUT', error)
    }
    throw new LinkPreviewError(
      `Failed to fetch URL: ${error instanceof Error ? error.message : 'Unknown error'}`,
+      'FETCH_FAILED',
      error
    )
  } finally {
@@ -230,9 +354,15 @@ async function fetchOEmbedData (

    if (oembedUrl === null) return null

+    // Validate discovered/provider oEmbed URL to prevent SSRF.
+    validateUrl(oembedUrl)
    ctx.info('fetching oEmbed data', { oembedUrl })

-    const response = await fetchWithTimeout(oembedUrl, { headers: { 'User-Agent': OEMBED_SERVICE_NAME } }, timeoutMs)
+    const { response } = await fetchWithValidatedRedirects(
+      oembedUrl,
+      { headers: { 'User-Agent': OEMBED_SERVICE_NAME } },
+      timeoutMs
+    )

    if (!response.ok) {
      ctx.warn('oEmbed fetch failed', {
@@ -259,12 +389,16 @@ async function fetchOEmbedData (
    ctx.info('successfully fetched oEmbed data', { type: data.type })
    return data
  } catch (error) {
-    // Don't throw - oEmbed failure should fall back to OG parsing
+    // Security-related URL validation errors must still fail fast.
+    if (error instanceof LinkPreviewError && error.code === 'BLOCKED_URL') {
+      throw error
+    }
+
+    // Other oEmbed failures should fall back to OG parsing.
    ctx.warn('failed to fetch oEmbed data', {
      error: error instanceof Error ? error.message : String(error)
    })
-    // return null
-    throw error
+    return null
  }
 }

@@ -424,7 +558,11 @@ async function loadImageSize (ctx: MeasureContext, url: string, config: Config):
    // Validate the image URL too
    validateUrl(url)

-    const response = await fetchWithTimeout(url, { headers: { 'User-Agent': config.UserAgent } }, timeoutMs)
+    const { response } = await fetchWithValidatedRedirects(
+      url,
+      { headers: { 'User-Agent': config.UserAgent } },
+      timeoutMs
+    )

    if (!response.ok) {
      ctx.warn('failed to fetch image', { status: response.status, url })
@@ -598,10 +736,9 @@ export async function parseLinkPreviewDetails (
  const parsedUrl = validateUrl(query)

  // Fetch the page
-  const response = await fetchWithTimeout(
+  const { response, finalUrl } = await fetchWithValidatedRedirects(
    query,
    {
-      redirect: 'follow',
      headers: {
        Accept: 'text/html,application/xhtml+xml',
        'User-Agent': config.UserAgent
@@ -617,10 +754,10 @@ export async function parseLinkPreviewDetails (
  // Check if response is an image
  const contentType = response.headers.get('content-type') ?? ''
  if (contentType.startsWith('image/')) {
-    const size = await loadImageSize(ctx, query, config)
+    const size = await loadImageSize(ctx, finalUrl, config)
    return {
-      url: query,
-      image: query,
+      url: finalUrl,
+      image: finalUrl,
      host: `${parsedUrl.protocol}//${parsedUrl.host}`,
      hostname: parsedUrl.hostname,
      imageWidth: size?.width,
@@ -639,17 +776,17 @@ export async function parseLinkPreviewDetails (
  const host = `${parsedUrl.protocol}//${parsedUrl.host}`

  // Try oEmbed first
-  const oembedData = await fetchOEmbedData(ctx, $, query, timeoutMs)
+  const oembedData = await fetchOEmbedData(ctx, $, finalUrl, timeoutMs)
  if (oembedData !== null) {
    const ogSiteName = $('meta[property="og:site_name"]').attr('content')
    const hostname = isNonEmptyString(ogSiteName) ? ogSiteName : parsedUrl.hostname
-    ctx.info('using oEmbed data', { url: query })
-    return convertOEmbedToPreview(oembedData, query, hostname, host)
+    ctx.info('using oEmbed data', { url: finalUrl })
+    return convertOEmbedToPreview(oembedData, finalUrl, hostname, host)
  }

  // Fall back to Open Graph / meta tag parsing
-  ctx.info('using Open Graph data', { url: query })
-  const preview = parseOpenGraphData($, config, parsedUrl, query)
+  ctx.info('using Open Graph data', { url: finalUrl })
+  const preview = parseOpenGraphData($, config, parsedUrl, finalUrl)

  // Get image dimensions if we have an image but no dimensions
  let imageWidth: number | undefined