Skip to content

Commit 730674e

Browse files
author
Waleed Latif
committed
fix(security): close ReDoS, zip-bomb, SSRF and redaction gaps found by audit
A dependency re-audit asked "does this library actually deliver the guarantee the calling code assumes" rather than "is it popular". These are the results. Security: - Copilot VFS glob() handed a caller-controlled pattern to micromatch, whose picomatch backend compiles `*` into a backtracking regex. A 25-char pattern took 41s; 29 chars exceeded 180s. Matching now runs on RE2, mirroring the grep() sibling in the same file that already did this. micromatch.makeRe() emits lookaheads RE2 cannot express, so picomatch's dot-assertions are translated into character exclusions; a differential test covers 519,688 pattern/path pairs against micromatch with no mismatches. Patterns that still carry an assertion fail closed rather than falling back. - doc-parser was the only OOXML-adjacent parser without the zip-bomb guard. officeparser sniffs content rather than extension, so a docx renamed .doc reached an unguarded unzip and could OOM the shared server. Genuine OLE2 .doc files are unaffected. Same guard added to the copilot style reader. - json-yaml-chunker called yaml.load with no alias-expansion cap. js-yaml has no maxAliasCount, so this is routed through assertYamlWithinLimits like the file parser already was. - Video tools and falai fetched provider-supplied URLs raw, including one that replayed an Authorization header to a polled status_url. Those URLs are now origin-pinned and revalidated on every poll. Cross-origin redirects drop credential headers by default, keyed on registrable domain rather than exact origin so same-site subdomain hops and http->https upgrades keep working. - Redaction key patterns were fully anchored, so openai_api_key, x-api-key, set-cookie, secretAccessKey and session_id were logged in the clear. The matcher now tokenizes keys, with exemptions so token-usage fields such as promptTokens stay readable. - drawtext escaping did not match ffmpeg's quoting grammar; a quote in agent supplied text escaped into filter options. Text is passed via textfile= so it never enters the filter string. - Loop conditions interpolated a resolved value into a JS string by hand. Supply chain: - Vendor free-email-domains' data. Its postinstall fetched a CDN CSV and overwrote the shipped list, so the lockfile hash covered the tarball but not the data actually used. It is inert under Bun today only because the package is outside trustedDependencies. - Document why xlsx is pinned to the SheetJS CDN: the npm copy is frozen at 0.18.5 with two unfixed CVEs, and a URL dependency is invisible to audit tooling, so upgrades have to be tracked by hand. Dependencies: - @daytonaio/sdk is deprecated in favour of @daytona/sdk (same API). - The @react-email/components 0.x line is deprecated; 1.0.12 needs no source changes here since no template uses <Tailwind> and render() already moved. - Load the pptx preview lazily so echarts leaves the Home, Files and share page bundles. - Drop autoprefixer (absent from the PostCSS config, so it never ran) and tailwind-merge (no importers; @sim/emcn declares its own).
1 parent 79b1ed1 commit 730674e

31 files changed

Lines changed: 7063 additions & 276 deletions

apps/sim/app/api/tools/onedrive/upload/route.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,17 @@
11
import { createLogger } from '@sim/logger'
22
import { getErrorMessage } from '@sim/utils/errors'
33
import { type NextRequest, NextResponse } from 'next/server'
4+
/**
5+
* `xlsx` is pinned in `package.json` to the SheetJS CDN tarball
6+
* (`https://cdn.sheetjs.com/xlsx-0.20.3/xlsx-0.20.3.tgz`), not the npm registry.
7+
* This is deliberate: npm's `xlsx` is abandoned at 0.18.5 (2022) and permanently
8+
* vulnerable to CVE-2023-30533 (prototype pollution) and CVE-2024-22363 (ReDoS)
9+
* — SheetJS shipped the fixes only to their own CDN and never republished to npm.
10+
* Do not "fix" this back to a registry version; that reintroduces both CVEs.
11+
*
12+
* A URL dependency is invisible to Dependabot and `bun audit`, so upgrades must
13+
* be tracked by hand against https://cdn.sheetjs.com/ / the SheetJS changelog.
14+
*/
415
import * as XLSX from 'xlsx'
516
import { onedriveUploadContract } from '@/lib/api/contracts/tools/microsoft'
617
import { parseRequest } from '@/lib/api/server'

apps/sim/app/api/tools/video/route.ts

Lines changed: 129 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ import { videoProviders, videoToolContract } from '@/lib/api/contracts/tools/med
88
import { getValidationErrorMessage, parseRequest, validationErrorResponse } from '@/lib/api/server'
99
import { checkInternalAuth } from '@/lib/auth/hybrid'
1010
import { getMaxExecutionTimeout } from '@/lib/core/execution-limits'
11+
import {
12+
secureFetchWithPinnedIP,
13+
validateUrlWithDNS,
14+
} from '@/lib/core/security/input-validation.server'
1115
import {
1216
assertKnownSizeWithinLimit,
1317
DEFAULT_MAX_ERROR_BODY_BYTES,
@@ -36,15 +40,31 @@ export const dynamic = 'force-dynamic'
3640
*/
3741
export const maxDuration = 5400
3842

39-
async function readVideoResponseBuffer(response: Response, label: string): Promise<Buffer> {
43+
/**
44+
* Structural shape shared by `fetch`'s `Response` and the SSRF-guarded
45+
* `SecureFetchResponse`, so the body readers below accept either.
46+
*/
47+
interface ReadableHttpResponse {
48+
ok: boolean
49+
status: number
50+
headers: { get(name: string): string | null }
51+
body?: ReadableStream<Uint8Array> | null
52+
arrayBuffer?: () => Promise<ArrayBuffer>
53+
text?: () => Promise<string>
54+
}
55+
56+
async function readVideoResponseBuffer(
57+
response: ReadableHttpResponse,
58+
label: string
59+
): Promise<Buffer> {
4060
return readResponseToBufferWithLimit(response, {
4161
maxBytes: MAX_VIDEO_OUTPUT_BYTES,
4262
label,
4363
})
4464
}
4565

4666
async function readVideoJson<T = Record<string, unknown>>(
47-
response: Response,
67+
response: ReadableHttpResponse,
4868
label: string
4969
): Promise<T> {
5070
return readResponseJsonWithLimit<T>(response, {
@@ -53,13 +73,54 @@ async function readVideoJson<T = Record<string, unknown>>(
5373
})
5474
}
5575

56-
async function readVideoErrorText(response: Response, label: string): Promise<string> {
76+
async function readVideoErrorText(response: ReadableHttpResponse, label: string): Promise<string> {
5777
return readResponseTextWithLimit(response, {
5878
maxBytes: DEFAULT_MAX_ERROR_BODY_BYTES,
5979
label,
6080
}).catch(() => '')
6181
}
6282

83+
/**
84+
* Fetches a provider-supplied URL through the SSRF-guarded client. These URLs come out of
85+
* provider response bodies, so they are untrusted input: DNS is resolved and validated
86+
* against the private/reserved policy, the socket is pinned to the validated IP (no DNS
87+
* rebinding), and every redirect hop is revalidated before it is followed.
88+
*/
89+
async function secureGetFromUntrustedUrl(
90+
url: string,
91+
paramName: string,
92+
options: { headers?: Record<string, string>; maxResponseBytes: number }
93+
) {
94+
const validation = await validateUrlWithDNS(url, paramName)
95+
if (!validation.isValid || !validation.resolvedIP) {
96+
throw new Error(validation.error || `${paramName} failed validation`)
97+
}
98+
return secureFetchWithPinnedIP(url, validation.resolvedIP, {
99+
method: 'GET',
100+
headers: options.headers,
101+
maxResponseBytes: options.maxResponseBytes,
102+
})
103+
}
104+
105+
/** Downloads a provider-supplied video URL through the SSRF-guarded client. */
106+
async function downloadVideoFromUrl(
107+
url: string,
108+
label: string,
109+
headers?: Record<string, string>
110+
): Promise<Buffer> {
111+
const response = await secureGetFromUntrustedUrl(url, 'videoUrl', {
112+
headers,
113+
maxResponseBytes: MAX_VIDEO_OUTPUT_BYTES,
114+
})
115+
116+
if (!response.ok) {
117+
await readVideoErrorText(response, `${label} error response`)
118+
throw new Error(`Failed to download video: ${response.status}`)
119+
}
120+
121+
return readVideoResponseBuffer(response, `${label} response`)
122+
}
123+
63124
export const POST = withRouteHandler(async (request: NextRequest) => {
64125
const requestId = generateId()
65126
logger.info(`[${requestId}] Video generation request started`)
@@ -461,14 +522,8 @@ async function generateWithRunway(
461522
throw new Error('No video URL in response')
462523
}
463524

464-
const videoResponse = await fetch(videoUrl)
465-
if (!videoResponse.ok) {
466-
await readVideoErrorText(videoResponse, 'Runway video error response')
467-
throw new Error(`Failed to download video: ${videoResponse.status}`)
468-
}
469-
470525
return {
471-
buffer: await readVideoResponseBuffer(videoResponse, 'Runway video response'),
526+
buffer: await downloadVideoFromUrl(videoUrl, 'Runway video'),
472527
width: dimensions.width,
473528
height: dimensions.height,
474529
jobId: taskId,
@@ -486,6 +541,21 @@ async function generateWithRunway(
486541
throw new Error('Runway generation timed out')
487542
}
488543

544+
/**
545+
* True when a provider-supplied URI is served by Google. The Veo download URI comes out of
546+
* the operation status body, so the `x-goog-api-key` credential is only attached when the
547+
* target really is a Google host — a hostile or spoofed URI never receives the key.
548+
*/
549+
function isGoogleApiHost(url: string): boolean {
550+
try {
551+
const { protocol, hostname } = new URL(url)
552+
if (protocol !== 'https:') return false
553+
return hostname === 'googleapis.com' || hostname.endsWith('.googleapis.com')
554+
} catch {
555+
return false
556+
}
557+
}
558+
489559
async function generateWithVeo(
490560
apiKey: string,
491561
model: string,
@@ -583,19 +653,12 @@ async function generateWithVeo(
583653
throw new Error('No video URI in response')
584654
}
585655

586-
const videoResponse = await fetch(videoUri, {
587-
headers: {
588-
'x-goog-api-key': apiKey,
589-
},
590-
})
591-
592-
if (!videoResponse.ok) {
593-
await readVideoErrorText(videoResponse, 'Veo video error response')
594-
throw new Error(`Failed to download video: ${videoResponse.status}`)
595-
}
596-
597656
return {
598-
buffer: await readVideoResponseBuffer(videoResponse, 'Veo video response'),
657+
buffer: await downloadVideoFromUrl(
658+
videoUri,
659+
'Veo video',
660+
isGoogleApiHost(videoUri) ? { 'x-goog-api-key': apiKey } : undefined
661+
),
599662
width: dimensions.width,
600663
height: dimensions.height,
601664
jobId: operationName,
@@ -697,14 +760,8 @@ async function generateWithLuma(
697760
throw new Error('No video URL in response')
698761
}
699762

700-
const videoResponse = await fetch(videoUrl)
701-
if (!videoResponse.ok) {
702-
await readVideoErrorText(videoResponse, 'Luma video error response')
703-
throw new Error(`Failed to download video: ${videoResponse.status}`)
704-
}
705-
706763
return {
707-
buffer: await readVideoResponseBuffer(videoResponse, 'Luma video response'),
764+
buffer: await downloadVideoFromUrl(videoUrl, 'Luma video'),
708765
width: dimensions.width,
709766
height: dimensions.height,
710767
jobId: generationId,
@@ -859,15 +916,8 @@ async function generateWithMiniMax(
859916
throw new Error('No download URL in file response')
860917
}
861918

862-
// Download the actual video file
863-
const videoResponse = await fetch(videoUrl)
864-
if (!videoResponse.ok) {
865-
await readVideoErrorText(videoResponse, 'MiniMax video error response')
866-
throw new Error(`Failed to download video from URL: ${videoResponse.status}`)
867-
}
868-
869919
return {
870-
buffer: await readVideoResponseBuffer(videoResponse, 'MiniMax video response'),
920+
buffer: await downloadVideoFromUrl(videoUrl, 'MiniMax video'),
871921
width: dimensions.width,
872922
height: dimensions.height,
873923
jobId: taskId,
@@ -1160,12 +1210,40 @@ function getFalAIErrorMessage(error: unknown): string {
11601210
return 'Unknown error'
11611211
}
11621212

1213+
const FALAI_QUEUE_ORIGIN = 'https://queue.fal.run'
1214+
11631215
function buildFalAIQueueUrl(
11641216
endpoint: string,
11651217
requestId: string,
11661218
path: 'response' | 'status'
11671219
): string {
1168-
return `https://queue.fal.run/${endpoint}/requests/${requestId}/${path}`
1220+
return `${FALAI_QUEUE_ORIGIN}/${endpoint}/requests/${requestId}/${path}`
1221+
}
1222+
1223+
/**
1224+
* Accepts a queue URL echoed back by Fal.ai only while it stays on the queue origin we
1225+
* submitted to, otherwise falls back to the URL we construct ourselves. Queue polling
1226+
* carries `Authorization: Key <apiKey>`, so an attacker-influenced `status_url` /
1227+
* `response_url` would otherwise hand the Fal.ai key to an arbitrary host.
1228+
*/
1229+
function resolveFalAIQueueUrl(candidate: string | undefined, fallback: string): string {
1230+
if (!candidate) return fallback
1231+
try {
1232+
return new URL(candidate).origin === FALAI_QUEUE_ORIGIN ? candidate : fallback
1233+
} catch {
1234+
return fallback
1235+
}
1236+
}
1237+
1238+
/**
1239+
* Requests a Fal.ai queue URL through the SSRF-guarded client. Invoked on every poll so
1240+
* the provider-supplied URL is revalidated each time rather than trusted once.
1241+
*/
1242+
async function fetchFalAIQueue(url: string, apiKey: string) {
1243+
return secureGetFromUntrustedUrl(url, 'falQueueUrl', {
1244+
headers: { Authorization: `Key ${apiKey}` },
1245+
maxResponseBytes: MAX_VIDEO_JSON_BYTES,
1246+
})
11691247
}
11701248

11711249
async function generateWithFalAI(
@@ -1218,7 +1296,7 @@ async function generateWithFalAI(
12181296
requestBody.generate_audio = generateAudio
12191297
}
12201298

1221-
const createResponse = await fetch(`https://queue.fal.run/${modelConfig.endpoint}`, {
1299+
const createResponse = await fetch(`${FALAI_QUEUE_ORIGIN}/${modelConfig.endpoint}`, {
12221300
method: 'POST',
12231301
headers: {
12241302
Authorization: `Key ${apiKey}`,
@@ -1242,12 +1320,14 @@ async function generateWithFalAI(
12421320
throw new Error('Fal.ai queue response missing request_id')
12431321
}
12441322

1245-
const statusUrl =
1246-
getStringProperty(createData, 'status_url') ||
1323+
const statusUrl = resolveFalAIQueueUrl(
1324+
getStringProperty(createData, 'status_url'),
12471325
buildFalAIQueueUrl(modelConfig.endpoint, requestIdFal, 'status')
1248-
const responseUrl =
1249-
getStringProperty(createData, 'response_url') ||
1326+
)
1327+
const responseUrl = resolveFalAIQueueUrl(
1328+
getStringProperty(createData, 'response_url'),
12501329
buildFalAIQueueUrl(modelConfig.endpoint, requestIdFal, 'response')
1330+
)
12511331

12521332
logger.info(`[${requestId}] Fal.ai request created: ${requestIdFal}`)
12531333

@@ -1258,11 +1338,7 @@ async function generateWithFalAI(
12581338
while (attempts < maxAttempts) {
12591339
await sleep(pollIntervalMs)
12601340

1261-
const statusResponse = await fetch(statusUrl, {
1262-
headers: {
1263-
Authorization: `Key ${apiKey}`,
1264-
},
1265-
})
1341+
const statusResponse = await fetchFalAIQueue(statusUrl, apiKey)
12661342

12671343
if (!statusResponse.ok) {
12681344
await readVideoErrorText(statusResponse, 'Fal.ai status error response')
@@ -1282,13 +1358,9 @@ async function generateWithFalAI(
12821358

12831359
logger.info(`[${requestId}] Fal.ai generation completed after ${attempts * 5}s`)
12841360

1285-
const resultResponse = await fetch(
1286-
getStringProperty(statusData, 'response_url') || responseUrl,
1287-
{
1288-
headers: {
1289-
Authorization: `Key ${apiKey}`,
1290-
},
1291-
}
1361+
const resultResponse = await fetchFalAIQueue(
1362+
resolveFalAIQueueUrl(getStringProperty(statusData, 'response_url'), responseUrl),
1363+
apiKey
12921364
)
12931365

12941366
if (!resultResponse.ok) {
@@ -1309,11 +1381,7 @@ async function generateWithFalAI(
13091381
throw new Error('No video URL in response')
13101382
}
13111383

1312-
const videoResponse = await fetch(videoUrl)
1313-
if (!videoResponse.ok) {
1314-
await readVideoErrorText(videoResponse, 'Fal.ai video error response')
1315-
throw new Error(`Failed to download video: ${videoResponse.status}`)
1316-
}
1384+
const videoBuffer = await downloadVideoFromUrl(videoUrl, 'Fal.ai video')
13171385

13181386
let width = getNumberProperty(videoOutput, 'width') || 1920
13191387
let height = getNumberProperty(videoOutput, 'height') || 1080
@@ -1325,7 +1393,7 @@ async function generateWithFalAI(
13251393
}
13261394

13271395
return {
1328-
buffer: await readVideoResponseBuffer(videoResponse, 'Fal.ai video response'),
1396+
buffer: videoBuffer,
13291397
width,
13301398
height,
13311399
jobId: requestIdFal,

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/file-viewer.tsx

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import { DocxPreview } from './docx-preview'
1616
import { resolveFileCategory } from './file-category'
1717
import { ImagePreview } from './image-preview'
1818
import type { PdfDocumentSource } from './pdf-viewer'
19-
import { PptxPreview } from './pptx-preview'
2019
import { PreviewPanel, resolvePreviewType } from './preview-panel'
2120
import {
2221
PREVIEW_LOADING_OVERLAY,
@@ -33,6 +32,19 @@ const PdfViewerCore = dynamic(() => import('./pdf-viewer').then((m) => m.PdfView
3332
ssr: false,
3433
})
3534

35+
/**
36+
* Lazy so `echarts` (~330 KB gzip, reached via `pptx-sandbox-host` →
37+
* `lib/pptx-renderer/renderer/chart-renderer`) stays out of every bundle that
38+
* statically imports this viewer - the Files page, the Home view and the public
39+
* share page - instead of only the visitors who open a PowerPoint file. The
40+
* fallback matches {@link PptxPreview}'s own pre-fetch frame, so the chunk load
41+
* and the binary fetch look like one continuous loading state.
42+
*/
43+
const PptxPreview = dynamic(() => import('./pptx-preview').then((m) => m.PptxPreview), {
44+
ssr: false,
45+
loading: () => <PreviewLoadingFrame className='h-full flex-1' tone='surface' />,
46+
})
47+
3648
const RichMarkdownEditor = dynamic(
3749
() => import('./rich-markdown-editor/rich-markdown-editor').then((m) => m.RichMarkdownEditor),
3850
{ ssr: false, loading: () => <PreviewLoadingFrame className='flex flex-1 flex-col' /> }

0 commit comments

Comments
 (0)