search

cure53 / DOMPurify

DOMPurify - a DOM-only, super-fast, uber-tolerant XSS sanitizer for HTML, MathML and SVG. DOMPurify works with a secure default, but offers a lot of configurability and hooks. Demo:
https://cure53.de/purify
Other
13.61k stars 695 forks source link

Unknown element should never be considered as void #886

Closed JounQin closed 8 months ago

JounQin commented 8 months ago

This issue proposes a bug

Background & Context

https://github.com/cure53/DOMPurify/blob/main/test/fixtures/expect.mjs#L886

Bug

Input


a<svg><xss><desc><noscript>&lt;/noscript>&lt;/desc>&lt;s>&lt/s>&lt;style>&lt;a title=\"&lt;/style>&lt;img src onerror=alert(1)>\">

Given output


a<svg><desc></desc></svg>

Expected output

a<svg></svg>

The expected output.

Feature

The xss element should be considered containing all its following instead of void element which is how browser works.

cure53 commented 8 months ago

Nah, I don't agree. Clearing the content of unknown elements would severely affect harmless sanitization results. You can change the behavior via a hook and take care of that yourself, but we won't change anything in the core library, sorry.

JounQin commented 8 months ago

But this is how browser works?

cure53 commented 8 months ago

Might be, but it would harm legitimate and harmless content - so we cannot empty it.

JounQin commented 8 months ago

@cure53 Then I don't quite understand this case:

https://github.com/cure53/DOMPurify/blob/d1e4f216664cc9e1ba5498a00b7d88a9cbd4c7f0/test/fixtures/expect.mjs#L330-L333

Why harmless div is removed?

cure53 commented 8 months ago

Because of the <frameset>, which causes the DIV (and almost everything else) to be removed: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/frameset

JounQin commented 8 months ago

I'm still confused, why div must be removed due to frameset?

cure53 commented 8 months ago

Please read the link I sent you, it's all there :)

JounQin commented 8 months ago

@cure53

Sorry, but I did read, but I could just find Deprecated.

If you mean:

A frameset document has a element instead of a element. The elements are placed within the .

I think <body>${fragment} will make sure the output always inside body and harmless?

cure53 commented 8 months ago

FRAMESET is a special tag has everything being removed that cannot be in there or around it, that includes a DIV.

JounQin commented 8 months ago

@cure53

I think <body>${fragment} will make sure the output always inside body and harmless?

image

cure53 commented 8 months ago

Yes, again, please inform yourself about HTML, about what the body tag does, what the frameset element does - once you know that, you will see it all makes sense.