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

Request body sanitization #305

Closed
RealOrangeOne opened this issue Sep 7, 2018 · 10 comments
Closed

Request body sanitization #305

RealOrangeOne opened this issue Sep 7, 2018 · 10 comments
Labels

Comments

@RealOrangeOne
Copy link
Member

RealOrangeOne commented Sep 7, 2018

Request bodies are stored as plain text in the database. This is great for debugging, but not so useful in terms of security, as passwords are technically stored in the database as plain text.

Django supports cleaning credentials using https://github.com/django/django/blob/b9cf764be62e77b4777b3a75ec256f6209a57671/django/contrib/auth/__init__.py#L41. An implementation using this would be good.

@meequz
Copy link

meequz commented Oct 9, 2018

The fact credentials are stored as plain text should be highlighted in readme in the first lines.

@acu192
Copy link
Contributor

acu192 commented Dec 12, 2018

I just created a PR to disclose this issue: #321

@mtford90 @avelis

@nyanev
Copy link

nyanev commented Mar 18, 2019

@avelis Do you have plans to address this issue?

@avelis
Copy link
Collaborator

avelis commented Mar 18, 2019

@nyanev I personally don't have time to do it. Is it ready to close?

@auvipy auvipy closed this as completed Apr 23, 2019
@acu192
Copy link
Contributor

acu192 commented Apr 23, 2019

@auvipy Why was this closed?

@auvipy
Copy link
Contributor

auvipy commented Apr 23, 2019

As there are links to potential fixes. It would be great if anyone could install the master branch in a local setting and check this is still not fixed, then could be reopened again

@quantumpacket
Copy link

Can this please be reopened unless a fix has already been applied?

@acu192
Copy link
Contributor

acu192 commented May 29, 2019

I second this. (x1000)

Until this is fixed, it should be well-advertised. There is no benefit in closing this issue before fixing it.

For inspiration, see this story and then this story. Logging sensitive data is a real problem.

@RealOrangeOne
Copy link
Member Author

Looks like #322 contains a fix for this, which was released in https://github.com/jazzband/django-silk/releases/tag/3.0.2. Therefore installing a fix is definitely possible.

Exactly how we ensure people know about this fix is a different story. I'll work on that one.

@makimat
Copy link

makimat commented Oct 22, 2020

Since this issue seems to be resolved, I think you could remove the security note from the main README.md? That's at least misleading people to think they might better not to use django-silk at all.

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

No branches or pull requests

8 participants