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.

Sign up for GitHub

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

Jump to bottom

Sanitize username and chat messages using DOMpurify #64

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Sanitize username and chat messages using DOMpurify #64

wants to merge 1 commit into from

Conversation

VaibhavSaini19
Copy link
Member

@VaibhavSaini19 VaibhavSaini19 commented Oct 2, 2020 •
edited
Loading

Although the fields have been sanitized, being a completely client-side code, anyone with a knack of Js knowledge can modify the code and inject malicious script back again.

@VaibhavSaini19 VaibhavSaini19 linked an issue Oct 2, 2020 that may be closed by this pull request
Sanitizing scripts given as input from the text box. #48
Open
@gridhead gridhead added bug Something isn't working enhancement New feature or request hacktoberfest Contribute to the notion of open-source this October! labels Oct 3, 2020
gridhead
gridhead approved these changes Dec 1, 2020
View reviewed changes
Copy link
Member

@gridhead gridhead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Could you take a look at this? @sachin2912 @s-katte?

@s-katte
Copy link
Member

s-katte commented Dec 2, 2020

Looks good to me.

Could you take a look at this? @sachin2912 @s-katte?

sure

@s-katte
Copy link
Member

s-katte commented Dec 2, 2020

@VaibhavSaini19, I have gone through docs here, and as you can see, they recommend not to use {SAFE_FOR_TEMPLATES: true} unless we don't have any other options.
So, aren't there any options?

@VaibhavSaini19
Copy link
Member Author

@VaibhavSaini19, I have gone through docs here, and as you can see, they recommend not to use {SAFE_FOR_TEMPLATES: true} unless we don't have any other options.
So, aren't there any options?

Since we are using it to sanitizing the input given by the user instead of a custom template string, it is safe to use that attribute here

@gridhead
Copy link
Member

gridhead commented Dec 2, 2020

@s-katte, please state if you find #64 (comment) satisfactory, so that we can go ahead and merge this one as well.

@gridhead gridhead requested a review from s-katte December 2, 2020 05:43
@gridhead gridhead assigned gridhead and unassigned VaibhavSaini19 Dec 2, 2020
s-katte
s-katte approved these changes Dec 2, 2020
View reviewed changes
Copy link
Member

@s-katte s-katte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, that is fine by me then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@s-katte s-katte s-katte approved these changes

@gridhead gridhead gridhead approved these changes

Assignees

@gridhead gridhead

Labels
bug Something isn't working enhancement New feature or request hacktoberfest Contribute to the notion of open-source this October!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Sanitizing scripts given as input from the text box.
3 participants
@VaibhavSaini19 @s-katte @gridhead