Navigation Menu

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

Issue 341 - XSS in link text #350

Merged
merged 1 commit into from May 11, 2020
Merged

Conversation

xurble
Copy link

@xurble xurble commented May 1, 2020

This will apply escaping on link text which can currently inject XSS

Fixes #341

@nicholasserra
Copy link
Collaborator

What's up with the removed_text and removed_text_compat variable changes here? Did you run into an issue with the existing replacement?

@xurble
Copy link
Author

xurble commented May 4, 2020

Yeah, it inserts the removed text “HTML_REMOVED” when it takes out something bad.

Obviously that’s got an underscore in it so if you have more than one in a paragraph you get unwanted italics.

It broke a bunch of unit tests.

I changed the string inserted to something innocuous and then I change it back to “HTML_REMOVED” later on in the pipeline after the italics replacement has run.

It’s a little cheesy but it passes all the unit tests.

@nicholasserra
Copy link
Collaborator

Gotcha. I'll take a better look at this sometime this week. If you search the main code for md5, you'll see there's some similar things happening, but using an md5 hash as a temporary placeholder.

@nicholasserra
Copy link
Collaborator

I don't love the replacement here but it'll do for now. Passes tests and fixes the issue. Thanks!

@nicholasserra nicholasserra merged commit 1353bae into trentm:master May 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Filter bypass leading to XSS
2 participants