Character escaping is over-eager?

[icefoxen]

Just noticed that in some places (meta content strings, class names, etc) it seems to get a bit over-zealous about escaping non-alphanumeric characters. For example, spaces become  , dashes become -, etc. This seems like overkill, and might make life a bit harder for things like searching for particular strings in the HTML source. If you agree I'll happily try to figure out a PR to make it only do this in places where it's necessary.

Thanks a lot!

[bodil]

It currently just defers to the htmlescape crate for the encoding. I'm willing to entertain the idea of a less eager escape function, but on the other hand I'm also inclined to err on the side of caution where XSS is concerned. If you can convince me you're not introducing an attack vector, I'd take a PR. 😊

[icefoxen]

Thanks! I've dug into htmlescape a little bit in pursuit of this but need to do it more, I think. I'm just generating static HTML, so it hadn't occurred to me that this might be an attack vector; I'll do some research into it and see. Thanks!

[jonathanKingston]

It's certainly over eager however I'm not 100% confident right now that I know all the characters it should be escaping in all cases.

I'm getting:

<form action="&#x2F;cart&#x2F;add" class="buy&#x2D;form" method="post"></form>

Instead of:

<form action="/cart/add" class="buy-form" method="post"></form>

With the following:

html!(
  <form action="/cart/add" method="post" class="buy-form"></form>
)

My hunch is htmlescape is what you would want to run on event listener attributes and on everything else it's likely just escaping " and ' is enough. However I will check further.

[jonathanKingston]

I think the PR I have submitted should be fine as the values are always quoted.

I think most advice is a little overcautious to allow for unquoted strings which are very easy to break out.

I think there should be a follow up to re-apply the entity escaping or stricter for event handlers. (at least until the most lax usage can be deduced).

Sources:
https://wonko.com/post/html-escaping#escaping-3
https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.md#rule-2---attribute-escape-before-inserting-untrusted-data-into-html-common-attributes