[Pkg-mozext-maintainers] firefox-sage diff for Lenny fixing CVE-2009-4102

[Alan Woodland]

2009/12/11 Steffen Joeris <steffen.joeris at skolelinux.de>:
> Hi Alan
>
> Thanks for sending us this debdiff.
>> Attached is my proposed diff for Lenny. It takes the 'least changes'
>>  approach to fixing the problem, which isn't great long term. Having
>>  applied this patch it now passes the test feeds in both html/non-html
>>  mode:
>>
>> http://users.aber.ac.uk/ajw/new.rss
>> http://users.aber.ac.uk/ajw/newI.rss
>> http://users.aber.ac.uk/ajw/everything.atom (this is the test case from
>>  2006 which had a regression)
>>
>> Etch is somewhat different, and still has my original patch from the 2006
>>  vulnerability which means there is no regression and it also fixed one of
>>  the newer test cases.
>>
>> Etch actually seemed to pass all the test cases there, but I know at least
>>  the malicious link one would be exploitable with only a very small change
>>  to the feed. (The benign 'exploit' made a few assumptions about which
>>  version of FF/IW you're using, which caused an exception to be thrown part
>>  way through executing the exploit, before there is any indication of
>>  failure).
>>
>> Please can you review this and allow me to make an upload to
>>  stable-security? I'll provide a similar patch for etch shortly too.
> The patch looks good, but when looking at it, I was wondering whether it
> misses a few parts?
>
> Do we need to call entityEncode around
> this.simpleHtmlParser.parse(item.getContent()); (line 242)
I don't think the output of this needs to be entity encoded, I think
the HTML parser needs to be entity encoded, my understanding of it is
that it should let through 'safe' HTML in fact so entity encoding here
would break things. I'll have a check though that it's not possible to
fool the sanitizer by using entity encoding in some (as yet unreported
way).

> There are also a few more itemget*() calls, where I am unable to determine
> whether they are all plain user input, maybe easier for someone that uses
> firefox-sage.
I'll have another check over these before making any move.

> If you can determine that this is all we need, then please go ahead.
I realised as well that my URL filtering, which was designed to
prevent 'funny' URLs (URL might not be the right term?) has a problem
as it stands currently.

The aim was to ensure that URLs like
"data:text/html;base64,PHNjcmlwdD4K...." or "javascript:" aren't
permitted, but we still allow http://... ftp://... etc. the way I
wrote it yesterday would transform
"data:data:text/html;base64,PHNjcmlwdD4K..."  into
"data:text/html;base64,PHNjcmlwdD4K...", and so not close the hole
properly. I think the safest way is probably to completely drop
anything we don't trust and not modify anything we do allow through.
There's some collateral damage from this strategy as a whole (e.g. we
wouldn't allow 'strange but safe' URLs like edonkey:// or whatever
else you may have registered), but I think this way round is safer
than me writing a blacklist of bad things (I can only think of
javacsript: and data: off the top of my head as 'bad', but I'd bet on
there being at least several more right now and I've not even looked
at HTML 5 at all).

Alan