Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Security issue: Bugsnag koa logs request cookies & request body #1630

Open
villesau opened this issue Dec 31, 2021 · 4 comments
Open

Security issue: Bugsnag koa logs request cookies & request body #1630

villesau opened this issue Dec 31, 2021 · 4 comments
Labels
backlog We hope to fix this feature/bug in the future bug Confirmed bug

Comments

@villesau
Copy link

villesau commented Dec 31, 2021

Describe the bug

When error is reported, bugsnag Koa plugin reports also request cookies:

headers: requestInfo.headers,

And

headers: request.headers,

As you see, there's no filter what so ever when cookies are passed forward.

Steps to reproduce

  1. Setup Koa project with standard bugsnag setup
  2. Fire an error
  3. In the logged error you also see cookies

Environment

  • Bugsnag version: 7.14.1, plugin: 7.14.0
  • Server framework version (if any):
    • Koa: 2.13.1

The issue likely exists in other plugins too.

E: I also think that the request body is logged as well. With post requests that might contain users passwords and other credentials too. Very dangerous.

@villesau villesau changed the title Security issue: Bugsnag koa logs request cookies Security issue: Bugsnag koa logs request cookies & request body Dec 31, 2021
@xljones
Copy link
Contributor

xljones commented Jan 3, 2022

Hey @villesau, the request headers, and body are captured where available. This is by design.

You can see exactly what data is captured automatically here, and how to remove them from Bugsnag reports: https://docs.bugsnag.com/platforms/javascript/koa/automatically-captured-data/#request-information

password is a redacted key term by default so won't be included in reports, but you can add more terms to this array as described here: https://docs.bugsnag.com/platforms/javascript/koa/configuration-options/#redactedkeys

@xljones xljones closed this as completed Jan 3, 2022
@villesau
Copy link
Author

villesau commented Jan 3, 2022

@xander-jones You have quite dangerous defaults then. How on earth you think that logging cookies is a sane default?

@xljones
Copy link
Contributor

xljones commented Jan 3, 2022

For many developers, logging cookies may be a helpful link in debugging. Sensitive data shouldn't be stored in cookies strictly speaking, but in cases where it is required, yes I agree it ought to be redacted. I'd suggest this is an exception to the norm.

I don't think this is a security issue per se as this data is redactable, and all automatically captured data is documented. But I hear your concerns with regards defaults – I'll raise this with the team 👍

For future travellers; you can remove cookies from ever being sent in Koa, (or equivalently for any JS framework with Bugsnag in use where a request is captured) with redactedKeys as follows:

Bugsnag.start({
  apiKey: YOUR_BUGSNAG_API_KEY,
  plugins: [BugsnagPluginKoa],
  redactedKeys: [
    'cookie',       // exact match: "cookie"
    'access_token', // exact match: "access_token"
    /^password$/i,  // case-insensitive: "password", "PASSWORD", "PaSsWoRd"
    /^cc_/          // prefix match: "cc_number" "cc_cvv" "cc_expiry"
  ]
})

const app = new Koa()
// app setup excluded for brevity ...

app.use(
  router()
    .get('/cookies', (ctx, next) => {
        ctx.cookies.set("foo", "bar");
        ctx.bugsnag.notify(new Error("Cookies be gone!"))
    })
  // ...
)

results in the following reported to Bugsnag under the request tab:

"headers": {
  "cookie": "[REDACTED]"
}

@villesau
Copy link
Author

villesau commented Jan 4, 2022

Thank you for taking it forward. Cookie is probably the most sensitive field of all of them since in regular web apps it has the session information that gives access to all the users data.

One way to make Bugsnag more secure (with typescript at least) would be to make redactedKeys mandatory. This way you could enforce the user to acknowledge the existence of the field, even when you explicitly pass an empty array.

@xljones xljones reopened this Jan 4, 2022
@xljones xljones added backlog We hope to fix this feature/bug in the future bug Confirmed bug labels Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog We hope to fix this feature/bug in the future bug Confirmed bug
Projects
None yet
Development

No branches or pull requests

2 participants