mganss / HtmlSanitizer

Cleans HTML to avoid XSS attacks
MIT License
1.52k stars 198 forks source link

Sanitizer replacing allowed css properties [text-decoration] #397

Closed JordanBrobyn closed 1 year ago

JordanBrobyn commented 1 year ago

Hello,

Is there a way to change the way the sanitizer is converting CssProperties like text-decoration?

For example, if I include html elements like <p><span style="text-decoration: underline;">Testing underline</span></p>

With rules as followed:

sanitizer.AllowedTags.Add("p"); sanitizer.AllowedTags.Add("span"); sanitizer.AllowedCssProperties.Add("text-decoration");

Text-decoration is being sanitized. If I include sanitizer.AllowedCssProperties.Add("text-decoration-line"); the allowed decorator text-decoration will be converted to text-decoration-line: underline in this case.

"text-decoration-line" is not currently supported by many email clients, which is causing a problem for us. Is there a way to maintain text-decoration and prevent it from being converted to the unsupported type?

Thanks

mganss commented 1 year ago

Currently it seems you'll have to allow the other text-decoration-* properties as well, i.e. "text-decoration", "text-decoration-color", "text-decoration-line", "text-decoration-skip", "text-decoration-style".

JordanBrobyn commented 1 year ago

Allowing other properties will introduce additional text-decoration attributes to appear in the style, which is also not supported by mail clients like Outlook.

For example, if I include

sanitizer.AllowedCssProperties.Add("text-decoration");
sanitizer.AllowedCssProperties.Add("text-decoration-line");
sanitizer.AllowedCssProperties.Add("text-decoration-style");

The resulting html would be <p><span style="text-decoration-line: initial; text-decoration-line: underline;">Testing underline</span></p>

It needs to remain as the following work in outlook <p><span style="text-decoration: underline;">Testing underline</span></p>

Is there a specific reason text-decoration is being replaced and can I override it? I see there are EventHandlers I can attach to, but I cant seem to attach to the right one before text-decoration is removed.

mganss commented 1 year ago

If you allow all of the longhand properties, then the shorthand property text-decoration is kept. This might seem strange, but it's due to the way AngleSharp works.

JordanBrobyn commented 1 year ago

That did it, thanks for the advice and I appreciate your work done on this package. Its a life saver.