Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Security vulnerability with serveStatic #123

Closed
MaSchwarz opened this issue Jan 20, 2024 · 5 comments · Fixed by #124
Closed

Security vulnerability with serveStatic #123

MaSchwarz opened this issue Jan 20, 2024 · 5 comments · Fixed by #124

Comments

@MaSchwarz
Copy link

Hello,
i recently discovered this library and I'm loving it so far. Thanks for the great work. ❤️

While working with the serveStatic method I noticed a behavior that I would describe as a security vulnerability.

Here's a simplified version of my project structure:

my-project/
├── node_modules
├── static/
│   └── global.css
├── src/
│   └── index.ts
├── package.json
└── .env

Here's a simplified version of my src/index.ts file. It's basically just copied from the docs.

// src/index.ts
import { serveStatic } from '@hono/node-server/serve-static'

app.use('/static/*', serveStatic({ root: './' }))

After booting up the server I was able to get my CSS file by sending a GET request to http://localhost:3000/static/global.css, nice 👍.
To my surprise I was also able to get the content of my .env file by sending a GET request to http://localhost:3000/static/../.env. That seems like a security vulnerability to me. I would expect that the scope is restricted to the static folder. Am I missing something?

I'm happy to provide further information and create a pull request.

@yusukebe
Copy link
Member

Hi @MaSchwarz

Thank you for raising the issue.

It is strange because we are taking action on that issue. If it is a bug, it is a vulnerability and needs to be fixed. Can you create a project that can reproduce it? Also, is it running on macOS, Windows, or Linux?

@MaSchwarz
Copy link
Author

MaSchwarz commented Jan 21, 2024

Thanks for your quick response.

I'm running on an M1 Mac with the latest OS version.

I created an example project here. You can run the server with npm run start.
While creating this simple example, I noticed that the issue is only happening when I send the request using Postman. I did add npm run fetch:env to send a cURL request and it works like it should. I tried to look for differences between the cURL request and the Postman one, but could not find any.

Edit: I did some further tinkering and added a simple console.log statement to the beginning of the serveStatic function. The console.log statement is visible on a Postman request but not on a cURL one. This leads to the conclusion that the serveStatic method is not even executed on a cURL requests, but is on a Postman one.

// serve-static.ts
...

export const serveStatic = (options: ServeStaticOptions = { root: '' }): MiddlewareHandler => {
  return async (c, next) => {
    console.log("I was here") // <-----------------------------
  
    // Do nothing if Response is already set
    if (c.finalized) return next()

...

@yusukebe
Copy link
Member

@MaSchwarz

Thank you so much!

I understoond what happened and will fix it.

@yusukebe
Copy link
Member

yusukebe commented Jan 21, 2024

@MaSchwarz !

This was caused by the new Request object introduced in v1.3.0 not resolving URLs properly. Fixed now and released v1.4.1 including the change.

Since this is a major vulnerability, we have issued an advisory.

GHSA-rjq5-w47x-x359

Thank you for the report. We appreciate it.

@MaSchwarz
Copy link
Author

I upgraded the dependency to v.1.4.1 in my project and can verify that it's fixed 🎉
@yusukebe Thanks for the quick and clean fix.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants