Skip to content

Commit b470278

Browse files
yusukebeyusukebe
authored
Merge commit from fork
1 parent 9aff14b commit b470278

4 files changed

Lines changed: 90 additions & 10 deletions

File tree

‎src/helper/ssg/ssg.test.tsx‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -563,6 +563,40 @@ describe('saveContentToFile function', () => {
563563
expect(fsMock.writeFile).toHaveBeenCalledWith('static/html.htm', yamlContent) // extensionMap
564564
expect(fsMock.writeFile).toHaveBeenCalledWith('static/html.html', yamlContent) // default + extensionMap
565565
})
566+
567+
it('should reject writing files outside outDir via path traversal', async () => {
568+
await expect(
569+
saveContentToFile(
570+
Promise.resolve({
571+
routePath: '/../pwned',
572+
content: 'owned',
573+
mimeType: 'text/html',
574+
}),
575+
fsMock,
576+
'./static'
577+
)
578+
).rejects.toThrow('Path traversal detected')
579+
580+
expect(fsMock.mkdir).not.toHaveBeenCalled()
581+
expect(fsMock.writeFile).not.toHaveBeenCalled()
582+
})
583+
584+
it('should reject paths that only partially match outDir name', async () => {
585+
await expect(
586+
saveContentToFile(
587+
Promise.resolve({
588+
routePath: '/../static-evil/pwned',
589+
content: 'owned',
590+
mimeType: 'text/html',
591+
}),
592+
fsMock,
593+
'./static'
594+
)
595+
).rejects.toThrow('Path traversal detected')
596+
597+
expect(fsMock.mkdir).not.toHaveBeenCalled()
598+
expect(fsMock.writeFile).not.toHaveBeenCalled()
599+
})
566600
})
567601

568602
describe('Dynamic route handling', () => {

‎src/helper/ssg/ssg.ts‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,13 @@ import { getExtension } from '../../utils/mime'
66
import type { AddedSSGDataRequest, SSGParams } from './middleware'
77
import { SSG_CONTEXT, X_HONO_DISABLE_SSG_HEADER_KEY } from './middleware'
88
import { defaultPlugin } from './plugins'
9-
import { dirname, filterStaticGenerateRoutes, isDynamicRoute, joinPaths } from './utils'
9+
import {
10+
dirname,
11+
ensureWithinOutDir,
12+
filterStaticGenerateRoutes,
13+
isDynamicRoute,
14+
joinPaths,
15+
} from './utils'
1016

1117
const DEFAULT_CONCURRENCY = 2 // default concurrency for ssg
1218

@@ -49,17 +55,20 @@ const generateFilePath = (
4955
): string => {
5056
const extension = determineExtension(mimeType, extensionMap)
5157

58+
let filePath: string
5259
if (routePath.endsWith(`.${extension}`)) {
53-
return joinPaths(outDir, routePath)
60+
filePath = joinPaths(outDir, routePath)
61+
} else if (routePath === '/') {
62+
filePath = joinPaths(outDir, `index.${extension}`)
63+
} else if (routePath.endsWith('/')) {
64+
filePath = joinPaths(outDir, routePath, `index.${extension}`)
65+
} else {
66+
filePath = joinPaths(outDir, `${routePath}.${extension}`)
5467
}
5568

56-
if (routePath === '/') {
57-
return joinPaths(outDir, `index.${extension}`)
58-
}
59-
if (routePath.endsWith('/')) {
60-
return joinPaths(outDir, routePath, `index.${extension}`)
61-
}
62-
return joinPaths(outDir, `${routePath}.${extension}`)
69+
ensureWithinOutDir(outDir, filePath)
70+
71+
return filePath
6372
}
6473

6574
const parseResponseContent = async (response: Response): Promise<string | ArrayBuffer> => {

‎src/helper/ssg/utils.test.ts‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, expect, it } from 'vitest'
2-
import { dirname, joinPaths } from './utils'
2+
import { dirname, ensureWithinOutDir, joinPaths } from './utils'
33

44
describe('joinPath', () => {
55
it('Should joined path is valid.', () => {
@@ -27,9 +27,34 @@ describe('joinPath', () => {
2727
expect(joinPaths('a\\b\\c', 'd\\e')).toBe('a/b/c/d/e')
2828
})
2929
})
30+
3031
describe('dirname', () => {
3132
it('Should dirname is valid.', () => {
3233
expect(dirname('parent/child')).toBe('parent')
3334
expect(dirname('windows\\test.txt')).toBe('windows')
3435
})
3536
})
37+
38+
describe('ensureWithinOutDir', () => {
39+
it('Should not throw for paths within outDir', () => {
40+
expect(() => ensureWithinOutDir('./static', 'static/index.html')).not.toThrow()
41+
expect(() => ensureWithinOutDir('./static', 'static/sub/page.html')).not.toThrow()
42+
expect(() => ensureWithinOutDir('/out', '/out/index.html')).not.toThrow()
43+
expect(() => ensureWithinOutDir('./static', 'static/a/../b.html')).not.toThrow()
44+
})
45+
46+
it('Should throw for paths outside outDir via traversal', () => {
47+
expect(() => ensureWithinOutDir('./static', 'pwned.txt')).toThrow('Path traversal detected')
48+
expect(() => ensureWithinOutDir('./static', '../pwned.txt')).toThrow('Path traversal detected')
49+
expect(() => ensureWithinOutDir('./out', 'pwned.txt')).toThrow('Path traversal detected')
50+
expect(() => ensureWithinOutDir('./static', 'static/../../pwned.txt')).toThrow(
51+
'Path traversal detected'
52+
)
53+
})
54+
55+
it('Should throw for paths that partially match outDir name', () => {
56+
expect(() => ensureWithinOutDir('./static', 'static-evil/pwned.html')).toThrow(
57+
'Path traversal detected'
58+
)
59+
})
60+
})

‎src/helper/ssg/utils.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,3 +73,15 @@ export const filterStaticGenerateRoutes = <E extends Env>(
7373
export const isDynamicRoute = (path: string): boolean => {
7474
return path.split('/').some((segment) => segment.startsWith(':') || segment.includes('*'))
7575
}
76+
77+
export const ensureWithinOutDir = (outDir: string, filePath: string): void => {
78+
const normalizedOutDir = joinPaths('/', outDir)
79+
const normalizedFilePath = joinPaths('/', filePath)
80+
81+
if (
82+
normalizedFilePath !== normalizedOutDir &&
83+
!normalizedFilePath.startsWith(`${normalizedOutDir}/`)
84+
) {
85+
throw new Error(`Path traversal detected: "${filePath}" is outside the output directory`)
86+
}
87+
}

0 commit comments

Comments
 (0)