Email with name incorrectly filtered by XSS Clean

[pijulius]

Hi Guys,

It seems the latest changes in the Security.php broke the email validation and so this way the emails with names became invalid emails.

For e.g.: Your Name <your@email.com>

will become: Your Name <your>

after a submit.

I tested this on both latest CI 3.0.3 and old CI2 and both have this issue and of course this wasn't the case before the last update on both CI2 and CI3 either.

I also know we can disable xss to make it work but that would be the wrong solution in my opinion, not to mention this shouldn't be filtered by xss anyway as it's not an invalid tag nor does it have any harmful content so it should be working as it was just before the latest releases.

Let me know if you need any more information on it, I see it has to to with xss_clean() function at "Sanitize naughty HTML elements" part where it wrongly parses the email and sets "email.com" as attribute and "your" as the tagname.

Regards, J

[narfbg]

When parsed as an HTML tag, it is an "your" tag with "email.com" attribute.

[pijulius]

Thanks for the quick "won't fix" answer :) but if you check out the wikipedia on email: https://en.wikipedia.org/wiki/Email_address

To indicate the message recipient, an email address also may have an associated display name for the recipient, which is followed by the address specification surrounded by angled brackets, for example: John Smith <john.smith@example.org>.

so this is a valid email address and you wrongly parse it as html tag (which it isn't) but thanks again.

Regards, J

[narfbg]

I'm perfectly well-aware of the named email format, thank you.

But when you put that as it is on an HTML page, it gets rendered as HTML - that's what XSS exploits and that's what XSS filters deal with. It doesn't matter what you (a human) know it is, what matters is how the browser interpets it.

And that's one of the downsides of "magic" do-it-all-in-one-function call XSS filters - if they guarantee security, there will be false-positives. If you just want to display "Foo bar@baz.com" as text, you can use the simpler html_escape() function and it would be just fine. But you can do that because you know the context, which xss_clean() doesn't.

[narfbg]

... for the record, I did write "Foo <bar@baz.com>" above.

You can now see that even GitHub parses it differently, which only exemplifies what I was talking about.

[pijulius]

Hi There,

You misunderstood my whole note :)

I don't use that to display it on a website, I use it to store the "Name <email@domain.com>" value in the db and then use it to send emails from this address so it has nothing to do with displaying it anywhere on the website so indeed this is a false positive and could be easily fixed (as it was working just fine before the last release) and it has nothing to do with html nor with github or anyone else :)

Regards, J

[narfbg]

If you're not using it in HTML (or JavaScript) context, then you shouldn't be passing it through an XSS filter. Especially if you do it before inserting into the database - never filter input, only output.

[pijulius]

I'm not putting in any xss filter, I'm using xss filter globally and only disable xss for specified fields that's why the whole problem.

[narfbg]

https://github.com/bcit-ci/CodeIgniter/blob/3.0.3/application/config/config.php#L421

[pijulius]

Ooo, I see now, hmm, won't give suggestions on what road CI3 takes but if I may share my opinion, that option is very very useful and so I don't think it should be deprecated as (know from experience with working a lot of people) that 90% of the programmers today (just imagine what kind of programmers there will be in the future if they even teach programming in prisons :) ) they will simply not know that xss needs to be applied to all data that comes in from user input and so a lot of websites will end up with xss issues just because one isn't thinking/knowing about this and that's why I prefer the other way around, have xss all over the website and you just allow with lets say "xss_allow" filter in the form_validation config file and that way you can be sure that if one allowed xss for a user input one did think about it why he did that and this way I find it a lot more useful/secure.

Anyway, thanks for your notes and really hope you will rethink the global xss filter.

Regards, J