MindscapeHQ/raygun4php

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

Closed this issue · 6 comments

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.

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

Awesome, thanks Callum :)

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

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.

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)).

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.