search

LaraBug / LaraBug

Laravel error reporting tool
https://www.larabug.com
MIT License
267 stars 58 forks source link

Added more sensitive parameter names to blacklist #53

Closed cbl closed 3 years ago

Cannonb4ll commented 3 years ago

Can you also add csrf_token? @SebastiaanKloos @Glennmen If you have any other suggestions to blacklist for keys we do not want to hold let me know.

cbl commented 3 years ago

@Cannonb4ll token is added so csrf_token will be filtered.

cbl commented 3 years ago

Oh no that is not true, thought a parameter will be filtered when it contains any of the words in blacklist.

Maybe add this behaviour? 🤔

Cannonb4ll commented 3 years ago

What I meant was, the csrf_token is an input hidden in laravel applications with a key called _token which we want to filter out too.

Glennmen commented 3 years ago

Things like email or similar information? To avoid any privacy issues.

cbl commented 3 years ago

@Cannonb4ll Yes I know. I added this to the other pr:

https://github.com/LaraBug/LaraBug/blob/5a462a9821b7299e4a53acadcbbf89375e909014/src/LaraBug.php#L223-L227

By this we can add string like *token* or *password* to the blacklist. So *token* would also filter csrf_token.

cbl commented 3 years ago

Things like email or similar information? To avoid any privacy issues.

I think email is something you might want to debug.

Cannonb4ll commented 3 years ago

@cbl I missed that one, yes, then you can use wildcards which is great.

The email and name values are a issue as well I recon, might want to obfuscate smartly before sending to larabug?

cbl commented 3 years ago

I think email and name is not something you want to filter. And if you want to filter it you have the option to do so. The reason I added parameters is because I it is usefull to see those values for debugging. You should never give access to the exceptions to anyone that should not have access to private data of your application anyway.

Glennmen commented 3 years ago

Why won't you want to filter it by default? What use does user information have for exception logging? Problem is not access but sending that data to a third party, Larabug in this case.

From my experience with other exception tools, mainly ones for Android (Crashlytics), by default they never send user information unless you add it yourself.

cbl commented 3 years ago

Yes, you are right. This data should not be passed on to third parties by default. I will add this to the blacklist.

Cannonb4ll commented 3 years ago

@Glennmen We really need to fix those tests.

@cbl Can you replace *username* for *name*? Its more broad.

Glennmen commented 3 years ago

I will try to look into it but this workflow wasn't made by me. I only did it for the Larabug app. But it shouldn't be blocking for this PR, I will fix it in a separate branch.