Skip to content

Setup Angular Rails XSS Escaping #1

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

Merged
merged 1 commit into from
Apr 20, 2018
Merged

Setup Angular Rails XSS Escaping #1

merged 1 commit into from
Apr 20, 2018

Conversation

ksylvest
Copy link
Contributor

@ksylvest ksylvest commented Apr 19, 2018

This library patches CGI.escapeHTML to replace {{ and }} with escaped versions. The reason for doing this is user generated input (i.e. form submissions, active record interpolation, etc.) can easily be spoofed to contain Angular interoperable blocks “{{2 + 3}}”. The style of injection might lead to XSS attacks.

@ksylvest ksylvest force-pushed the initial-setup branch 3 times, most recently from d8eca19 to de103b2 Compare April 19, 2018 19:15
@ksylvest ksylvest requested a review from claudioclutter April 19, 2018 19:20
@ksylvest
Copy link
Contributor Author

@claudioclutter for review - integration with specs is here: https://github.com/clutter/clutter-platform/pull/3570

@ksylvest ksylvest force-pushed the initial-setup branch 2 times, most recently from aa79c88 to f9e9862 Compare April 19, 2018 19:26
expect(escaped).to eql('{{"\\{\\{"{{"\\}\\}"}}2+3{{"\\}\\}"}}')
end

it 'also does the normal CG.escapeHTML to work with other escapes' do
Copy link
Contributor

Choose a reason for hiding this comment

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

@ksylvest Minor typo: CG vs CGI

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks - I also hate the new Macbook keyboard...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixup: c0b9742

README.md Outdated
```
## Dependencies

WT is tested with Rails 5.2.0 and Ruby 2.5.1.
Copy link
Contributor

Choose a reason for hiding this comment

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

@ksylvest WT here

Copy link
Contributor

Choose a reason for hiding this comment

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

Also… we are on Rails 5.1, so I guess it's tested there too 😊

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixup: c0b9742 (technically not tested on 5.2!)

@ksylvest
Copy link
Contributor Author

@claudioclutter sorry - I realized I assigned this a bit early. The XSS replacement had a bug with quote escaping - so opted to go with a $root variable and define it. This fixup has the changes: a717f91

Copy link
Contributor

@claudioclutter claudioclutter left a comment

Choose a reason for hiding this comment

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

security > beauty

😊

This library patches CGI.escapeHTML to replace {{ and }} with escaped versions. The reason for doing this is user generated input (i.e. form submissions, active record interpolation, etc.) can easily be spoofed to contain Angular interoperable blocks “{{2 + 3}}”. The style of injection might lead to XSS attacks.
@ksylvest ksylvest merged commit 89759d0 into master Apr 20, 2018
@ksylvest ksylvest deleted the initial-setup branch April 20, 2018 21:48
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.

2 participants