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

Do not parse link titles for IRC formatting #245

Merged
merged 1 commit into from
Apr 6, 2016
Merged

Do not parse link titles for IRC formatting #245

merged 1 commit into from
Apr 6, 2016

Conversation

xPaw
Copy link
Member

@xPaw xPaw commented Apr 4, 2016

This was supposed to be an XSS fix in 7ef2da0, but the proper fix would be to simply escape it, instead of running IRC formatter on it.

@maxpoulin64
Copy link
Member

Agreed, doesn't even make sense. 👍

@maxpoulin64 maxpoulin64 added Type: Feature Tickets that describe a desired feature or PRs that add them to the project. second review needed labels Apr 4, 2016
@astorije
Copy link
Member

astorije commented Apr 6, 2016

For the record, this was a fix for a pretty bad XSS vulnerability: erming/shout#455.
I remember when Shout had it, it messed up people's client for a while.

Are there any other consequences to this change?

Thanks for this, @xPaw! 👍

@astorije astorije merged commit 7024bd4 into master Apr 6, 2016
@astorije astorije deleted the xpaw/no-link-parse branch April 6, 2016 02:20
@astorije astorije added Type: Security Security concern or PRs that must be reviewed with extra care regarding security. and removed second review needed labels Apr 6, 2016
@maxpoulin64
Copy link
Member

@astorije No, no consequences really. We shouldn't be doing that. This makes it so it is strictly text, which is good. There's no reason to parse links and colors in web page titles, they're not meant to be displayed as such anyway.

The original code was {{{head}}} which allowed everything through. This one is {{head}}, which lets nothing through.

@astorije
Copy link
Member

astorije commented Apr 6, 2016

Agreed, thanks a lot for the details!

@astorije astorije added Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors. and removed Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors. labels Apr 13, 2016
@astorije astorije added this to the 1.5.0 milestone Sep 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@maxpoulin64 maxpoulin64

Labels
Type: Feature Tickets that describe a desired feature or PRs that add them to the project. Type: Security Security concern or PRs that must be reviewed with extra care regarding security.
Projects
None yet
Milestone
1.5.0
Development

Successfully merging this pull request may close these issues.
3 participants
@xPaw @maxpoulin64 @astorije