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

An alternative fix suggestion for legacy apps #5

Open
dalevink opened this issue Feb 28, 2018 · 4 comments
Open

An alternative fix suggestion for legacy apps #5

dalevink opened this issue Feb 28, 2018 · 4 comments

Comments

@dalevink
Copy link

dalevink commented Feb 28, 2018

I have a suggestion, for any existing app that consistently addresses existing XSS vulnerabilities (pre Vue).

For example, you (should?) have an existing "globally" used function, such as:

function htmlEscape($text) {
  return htmlspecialchars(strval($text), ENT_QUOTES, 'UTF-8');
}

Could this be simply altered to include the escaping of Vue template interpolation, eg:

function htmlEscape($text) {
  $text = str_replace("{", "{{ '{' }}", strval($text));
  return htmlspecialchars($text, ENT_QUOTES, 'UTF-8');
}

A possible one line fix?

Note: Updated code fix as per suggestion below – thanks to @apreiml

@apreiml
Copy link

apreiml commented Mar 6, 2018

@dalevink It would be better to escape even one '{' character. Otherwise XSS is still possible, if you concatenate output somewhere.

For example:

htmlEscape('{').htmlEscape('{ 5 + 5 }}')

@helmut
Copy link

helmut commented Mar 8, 2018

It would be great if the htmlspecialchars function would convert the curly brackets {} into entities like it does with the greater than/less than symbols <>.

Haven't had a chance to test it but that would make sense without adding extra spans throughout your layout.

UPDATE: Just tested and vue still interprets the entities! Thats interesting...

@dalevink
Copy link
Author

dalevink commented Mar 8, 2018

@helmut I found this and was surprised too that Vue interprets the entities!

Yeah, the extra spans aren't pretty, but they could be a temporary trade-off as you migrate to better methods.

Hold that thought… here’s a way to avoid ANY extra markup and simply use Vue itself to escape the string:
https://jsfiddle.net/dalevink/p9gj54Lo/16/
… that's nicer – I've updated the code above too :)

@apreiml
Copy link

apreiml commented Mar 9, 2018

@dalevink This is also a nice solution, but then you should be careful if you use this escape method outside of the Vue app scope (e.g. in the header).

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

No branches or pull requests

3 participants