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

PII/secrets leaked via exception message #512

Open
Lidbetter opened this issue Jan 15, 2021 · 2 comments
Open

PII/secrets leaked via exception message #512

Lidbetter opened this issue Jan 15, 2021 · 2 comments
Assignees
Labels
Rank: 3 - Minor Tackle when there are no actionable Critical or Major requests. Status: 2 - Accepted The scope and characteristics of the request are defined and ready to be worked. Type: Enhancement Changes that add to, improve upon, enhance, or extend the existing component.

Comments

@Lidbetter
Copy link

Lidbetter commented Jan 15, 2021

In a similar vein to the other open scrubbing issues, exception messages themselves leak information.

for example:

Illuminate\Database\QueryException: SQLSTATE[40001]: Serialization failure: 1213 Deadlock found when trying to get lock; try restarting transaction (SQL: UPDATE users SET password = $bcrypted_hash_here

Such an exception also causes issues when capturing args:

Illuminate\Database\Connection::Illuminate\Database\{closure} arguments
    0: "UPDATE users SET password = ?, WHERE id = ?"
    1: [  $bcrypted_hash_here,  __user_id_here__  ]

This is the most obvious clear cut example, but there are other more subtle ways information can be leaked via exception messages.

  • Is this something that can be addressed in the rollbar library?
  • Are there any suggested workarounds/mitigations we can implement in the meantime?
@bishopb
Copy link
Contributor

bishopb commented Jun 17, 2021

Thanks for the report! I can confirm I have also seen this in the wild, for example:

{
  "method": "Doctrine\\DBAL\\Driver\\PDOConnection::__construct", 
  "args": [
    "mysql:host=127.0.0.1;dbname=foxcatdb;charset=UTF8;", 
    "foxcatdb", 
    "Just-another-password-test", 
    []
  ], 
  "lineno": 44, 
  "filename": "/tmp/foxcat/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOMySql/Driver.php"
}, 

The immediate mitigation is to use the scrub_whitelist (soon to be called scrub_safelist) to pattern match against the keys and include only the ones that are known safe. It may take some fiddling to get right, depending upon the shape of the data.

There is a test that covers the scrubbing logic, but it could do with some expansion to demonstrate how it's possible to handle these particular shapes. I'd encourage you to try the scrublist method and attempt to black it out and submit a PR to demonstrate that in test. I'll also work on this.

@bishopb bishopb added Rank: 3 - Minor Tackle when there are no actionable Critical or Major requests. Status: 2 - Accepted The scope and characteristics of the request are defined and ready to be worked. Type: Enhancement Changes that add to, improve upon, enhance, or extend the existing component. labels Jun 17, 2021
@bishopb bishopb self-assigned this Jun 17, 2021
@bishopb
Copy link
Contributor

bishopb commented Jun 17, 2021

It occurs to me that a more surgical approach might be similar to how GitHub Advanced Security scans repositories looking for secrets: it does so by knowing the shape of the secrets and surrounding context. For example, anything of the form \W\$2[ayb]\$.{56}\W is likely a Bcrypt hash and could be replaced with *** or similar. Of course, the GHAS folks don't have the runtime performance constraints we do, so it may be unwise to allow regex scanning over the traces. (In particular, we'd want to avoid accidental or malicious regex that led to ReDOS.)

Ideas welcomed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Rank: 3 - Minor Tackle when there are no actionable Critical or Major requests. Status: 2 - Accepted The scope and characteristics of the request are defined and ready to be worked. Type: Enhancement Changes that add to, improve upon, enhance, or extend the existing component.
Projects
None yet
Development

No branches or pull requests

2 participants