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

PHP_AUTH_USER and PHP_AUTH_PW (BasicAuth credentials) being sent and stored in Raygun dashboard #65

Closed
madmatt opened this issue Apr 1, 2015 · 6 comments

Comments

@madmatt
Copy link

madmatt commented Apr 1, 2015

Hey team,

If the $_SERVER superglobal has PHP_AUTH_USER and/or PHP_AUTH_PW variables defined (because a user has logged in using basic auth), they get included and passed through to the Raygun dashboard.

These should be stripped out by default I think, perhaps with an optional config flag to allow users to not filter these (e.g. for cases where you need to store that information to replay requests correctly, although I don't think you should ever allow PHP_AUTH_PW to be stored).

This would probably be fixed by #32, and could form part of a default blacklist of fields.

fundead pushed a commit that referenced this issue Apr 17, 2015
Can be overridden by passing in true as the 5th constructor param

ref #65
@fundead
Copy link
Contributor

fundead commented Apr 17, 2015

Thanks for reporting this issue, I've implemented filtering by default for the two basic auth fields in $_SERVER, which will also be able to be disabled by passing in 'true' as the 5th param to the constructor. This provider's next in line for a release so this should be out soon.

Regards,

Callum Gavin
Mindscape Limited

@madmatt
Copy link
Author

madmatt commented Apr 17, 2015

Awesome, thanks Callum :)

@sminnee
Copy link

sminnee commented Feb 16, 2017

Related to this, it's increasingly common to use of environment variables to pass sensitive data such as database passwords or API keys into an application. If these end up in $_ENV I assume that they will also be passed through to raygun and be a security issue.

Blacklisting every variable your app uses would be possible, but a little bit error prone. Perhaps the black-list could be defined as a set of regexes instead of fixed strings? That being the case I would recommend the following as a default:

/(PWD|KEY|PASSWORD)/i

@madmatt
Copy link
Author

madmatt commented Nov 30, 2017

Hey again @fundead,

I'm keen to see if it would be okay for me to write a pull request that implements a similar default to what @sminnee suggests above? I'm aware that Mindscape provides a bunch of these repos for different languages and you probably want to keep feature parity across them, but I'm not sure how strict that requirement is?

Specifically, I'm suggesting adding a default regex that will filter (but not strip) any value where a key matches one of: /KEY|PASSWORD|PWD|PRIVATE|API/i. The idea behind filtering rather than removing is so that people can at least see that there was a value defined, but it hasn't be passed to Raygun to store.

@CmdrKeen
Copy link
Contributor

Hi Matt,

I suspect that @BenjaminHarding or @samuel-holt might be the goto here.

We do have some default filters in our other providers (thinking specifically of our Raygun4Ruby one here - both for types of errors and filtering keys). Super happy to have that added.

We're not adverse to differences, we are usually always slowly rolling out improvements across all providers and it takes time. We do have a goal of trying to achieve parity in features though :)

(on the off chance you're talking about init-ing via env values: I'd ask if you did make a PR that it be non-breaking (so current approach works, but your approach would also work)).

@GuySartorelli
Copy link
Contributor

I think this should be closed - the default behaviour for filtering those values can't be changed without breaking semver (assuming this project adheres to semver) and the ability to filter them out either with regex or exact names is already implemented.

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

No branches or pull requests

6 participants