[TYPO3-core] RFC: #15812: Add backend maintenance for login news

[Helmut Hummel]

Hi,

On 07.10.10 21:25, Jigal van Hemert wrote:
> 
> On 7-10-2010 18:56, Helmut Hummel wrote:
>>
>> 1. It is kind of unmaintained
> 
> True. It needs probably a combination of someone who is very much into 
> XSS problems and a programmer who can convert this knowledge in code.

Well for the latter, rsnake did a great job already:
http://ha.ckers.org/xss.html

But we still need the people who have the knowlege _and_ the time to do
this.

>> 1a. The code is difficult (because of the regex) which makes it hard (at
>> least for me) to understand what it really does.
> 
> I can only advise to invest some time in learning PCRE. As we've seen in 
> some performance related RFCs the code in the PCRE routines often 
> performs as well as or better than PHP functions which seem much simpler.

I would say, that I have a fair knowledge of PCRE and I understand the
rough concept how RemoveXSS works. Nevertheless I would not it's a
trivial and easy piece of code, thus I find it hard to maintain.

> It could be split up in separate functions to improve readability (and 
> to increase "code quality").

This might help, but I'm not sure.

>> EDIT: It's much better than I thought, but still blacklisting is error
>> prone and needs someone keeping the blacklist up to date.
> 
> There is an identical problem with white lists: a list of allowed 
> tags/keywords/properties must be updated when HTML5 support is 
> introduced, or with newer versions of JavaScript, CSS, etc.

But it is totally different security wise. The "allow all, deny some"
concept leaves you insecure if you forget "something". In a "allow none,
allow some" szenario, you need to adjust "something" if you want more
features, but at all times you are secure.

White listing is indubitable more secure than black listing.

>> 3. The tampering it does might be goodfor some (most?) cases, but the
>> result looks pretty ugly and it breaks the HTML output, produces invalid
>> HTML.
> 
> True. The tampering is very simple and can easily be modified. I guess 
> it was the choice between XSS and invalid HTML.

I understand that. Nevertheless it is not nice.

>> 4. Having said all this one last thing: It teaches a wrong lesson which
>> is: "Yeah, let's use RemoveXSS and wer're done with security".
> 
> Judging by the number of people who still inject external data directly 
> into a SQL query, I'd say we would be lucky if someone actually used 
> RemoveXSS :-/

We would have won nothing if they'd used RemoveXSS to prevent SQL
injections. But I agree that when it comes to HTML RemoveXSS is better
than nothing.

>> What we really need is HTML Purifier[1], which uses a whitelist
>> approach, always returns cleaned up and valid! HTML as a result and is
>> an open source project which is actively developed. I have it on my list
>> to get it into 4.5, but had no time to do it until now.
> 
> I am very curious about its performance.

Well I don't know. Judging from the feature set it looks much better to
me, and I like the fact that it is maintained by more than one person.

> The main reason why I modified 
> RemoveXSS in the first place was performance. It was used in an 
> extension and the thing was incredibly slow. Soon it was clear that 
> RemoveXSS contributed in a large way to the lack of speed.

To be honest. I don't care about performance if it comes to security.
That's where proper caching has to be implemented.

But I'm getting off-topic ;)

Regards Helmut