Skip to content

Commit

Permalink
don't allow directory traversal and fix the behavior when root includ…
Browse files Browse the repository at this point in the history
…ing `../`
  • Loading branch information
yusukebe committed Sep 22, 2024
1 parent 5dd59c5 commit a4ecbeb
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 15 deletions.
70 changes: 56 additions & 14 deletions src/middleware/serve-static/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,6 @@ describe('Serve Static Middleware', () => {

app.get('/static/*', serveStatic)

const serveStaticAbsoluteRoot = baseServeStatic({
getContent,
pathResolve: (path) => {
return path
},
root: '/home/hono/../foo',
})

app.get('/static-absolute/*', serveStaticAbsoluteRoot)

beforeEach(() => {
getContent.mockClear()
})
Expand Down Expand Up @@ -237,9 +227,61 @@ describe('Serve Static Middleware', () => {
expect(res.body).toBe(body)
})

it('Should traverse directories with absolute root path', async () => {
const res = await app.request('/static-absolute/bar/hello.html')
expect(res.status).toBe(200)
expect(await res.text()).toBe('Hello in /home/foo/static-absolute/bar/hello.html')
describe('Changing root path', () => {
const pathResolve = (path: string) => {
return path.startsWith('/') ? path : `./${path}`
}

it('Should return the content with absolute root path', async () => {
const app = new Hono()
const serveStatic = baseServeStatic({
getContent,
pathResolve,
root: '/home/hono/child',
})
app.get('/static/*', serveStatic)

const res = await app.request('/static/html/hello.html')
expect(await res.text()).toBe('Hello in /home/hono/child/static/html/hello.html')
})

it('Should traverse the directories with absolute root path', async () => {
const app = new Hono()
const serveStatic = baseServeStatic({
getContent,
pathResolve,
root: '/home/hono/../parent',
})
app.get('/static/*', serveStatic)

const res = await app.request('/static/html/hello.html')
expect(await res.text()).toBe('Hello in /home/parent/static/html/hello.html')
})

it('Should treat the root path includes .. as relative path', async () => {
const app = new Hono()
const serveStatic = baseServeStatic({
getContent,
pathResolve,
root: '../home/hono',
})
app.get('/static/*', serveStatic)

const res = await app.request('/static/html/hello.html')
expect(await res.text()).toBe('Hello in ./static/html/hello.html')

Check failure on line 271 in src/middleware/serve-static/index.test.ts

View workflow job for this annotation

GitHub Actions / Main

src/middleware/serve-static/index.test.ts > Serve Static Middleware > Changing root path > Should treat the root path includes .. as relative path

AssertionError: expected 'Hello in ./../home/hono/static/html/h…' to be 'Hello in ./static/html/hello.html' // Object.is equality Expected: "Hello in ./static/html/hello.html" Received: "Hello in ./../home/hono/static/html/hello.html" ❯ src/middleware/serve-static/index.test.ts:271:32
})

it('Should not allow directory traversal with . as relative path', async () => {
const app = new Hono()
const serveStatic = baseServeStatic({
getContent,
pathResolve,
root: '.',
})
app.get('*', serveStatic)

const res = await app.request('///etc/passwd')
expect(res.status).toBe(404)
})
})
})
4 changes: 3 additions & 1 deletion src/middleware/serve-static/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,10 @@ export const serveStatic = <E extends Env = Env>(
if (options.root) {
if (options.root.startsWith('/')) {
isAbsoluteRoot = true
root = new URL(`file://${options.root}`).pathname
} else {
root = options.root
}
root = new URL(`file://${options.root}`).pathname
}

return async (c, next) => {
Expand Down
4 changes: 4 additions & 0 deletions src/utils/filepath.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,9 @@ export const getFilePathWithoutDefaultDocument = (
let path = root ? root + '/' + filename : filename
path = path.replace(/^\.?\//, '')

if (root[0] !== '/' && path[0] === '/') {
return
}

return path
}

0 comments on commit a4ecbeb

Please sign in to comment.