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

Dumping entire request objects is unsafe #21

Open
eropple opened this issue Oct 7, 2019 · 2 comments
Open

Dumping entire request objects is unsafe #21

eropple opened this issue Oct 7, 2019 · 2 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@eropple
Copy link

eropple commented Oct 7, 2019

Shipping req objects as part of error reports can lead to the leakage of secure information, including credentials. Sentry will strip "sensitive" keys in a project, but 1) users have to know to turn it on, 2) this still means you're sending the content over the wire to a third party, and 3) they won't strip all of them; Authentication headers, for example, seem to be retained, and depending on the arbitrary naming of request body fields the contents thereof may or may not actually be stripped.

I'd suggest instead a whitelist of content to send, rather than just sending an entire req object (which also results in the serialization of less-than-useful content, too).

This is probably a major version iteration, but unfortunately, as-is, nest-raven is IMO unsafe in production. I may be able to do the heavy lifting on fixes, however, if we can agree on an API.

@mentos1386 mentos1386 added question Further information is requested enhancement New feature or request and removed question Further information is requested labels Oct 10, 2019
@eropple
Copy link
Author

eropple commented Oct 10, 2019

Hey--

So I did look at that before posting, and it seems like the keys that it extracts are shallow ones. And they make sense--pull headers, cookies, that sort of thing. Where it goes sideways is nested keys, like req.headers.authorization, which it doesn't seem to deal with (and relies on Sentry's remote sensitive-key stripping to deal with, where it can?).

I've learned from experience to lead people into making secure choices by default. Some degree of configurability here might help, but I'm still concerned overall with how easy it'll be to accidentally data-exfiltrate yourself.

@mentos1386 mentos1386 added this to the v7.0.0 milestone Apr 18, 2020
@mentos1386 mentos1386 added the help wanted Extra attention is needed label Apr 18, 2020
@9renpoto 9renpoto removed this from the v7.0.0 milestone Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants