mganss / HtmlSanitizer

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

Incorrect <Img> sanitize when source is empty or not URI #439

Closed andreyshiryaev closed 1 year ago

andreyshiryaev commented 1 year ago

Good day

<img src='http://url.to.file.which/not.exist1' onerror='alert(document.cookie)'> works fine

But I found 2 issues

  1. when i try to clear <img src='' onerror='alert(document.cookie)'> nothing happens i expect that to be deleted onerror='alert(document.cookie)'

I expect like <img src='''>

  1. when src is not URI <IMG SRC=j&#X41vascript:alert('test2')>

I expect like <img src='''>

mganss commented 1 year ago

Both cases seem to work fine for me. What do you expect as output in each case?

andreyshiryaev commented 1 year ago

I expect that onerror='alert(document.cookie)' and j&#X41vascript:alert('test2') will be removed.

I try to fix XSS image

mganss commented 1 year ago

Strange. Both work fine for me. Can you provide a short code example that shows the issue?

andreyshiryaev commented 1 year ago

Strange. Both work fine for me. Can you provide a short code example that shows the issue?

Program.zip

FIY the variant with empty src works in the console project for me too. It is my mistake sorry. It does not work only non-URI value

UPDATE maybe it happens with different text could you please give some advice on how to use sanitizer with such text from HTML editor? thanks

mganss commented 1 year ago

The case <IMG SRC=j&#X41vascript:alert('test2')> does not work in your demo because you have not included src in the UriAttributes collection. The UriAttributes collection determines the attributes which will be subject to URI sanitization.

I'm not sure I understand the <p><img>&lt;img src=''alert(document.cookie)'&gt;&nbsp;<img><img></p> case. What do you expect as output? There doesn't seem to be an img element that needs to be sanitized. The src="..." part is just text, it's not an attribute of an element.

andreyshiryaev commented 1 year ago

The case <IMG SRC=j&#X41vascript:alert('test2')> does not work in your demo because you have not included src in the UriAttributes collection. The UriAttributes collection determines the attributes which will be subject to URI sanitization.

I'm not sure I understand the <p><img>&lt;img src=''alert(document.cookie)'&gt;&nbsp;<img><img></p> case. What do you expect as output? There doesn't seem to be an img element that needs to be sanitized. The src="..." part is just text, it's not an attribute of an element.

Thanks a lot. Close the issue

andreyshiryaev commented 1 year ago

The case <IMG SRC=j&#X41vascript:alert('test2')> does not work in your demo because you have not included src in the UriAttributes collection. The UriAttributes collection determines the attributes which will be subject to URI sanitization. I'm not sure I understand the <p><img>&lt;img src=''alert(document.cookie)'&gt;&nbsp;<img><img></p> case. What do you expect as output? There doesn't seem to be an img element that needs to be sanitized. The src="..." part is just text, it's not an attribute of an element.

Thanks a lot. Close the issue. It is problem with the front side too. it send wrong data