[dev] [commits] Horde branch master updated. d255d66a6a8ace592e3a058e455f628cc6aa1faa

[Michael M Slusarz]

Quoting Chuck Hagenbuch <chuck at horde.org>:

> Quoting Michael M Slusarz <slusarz at horde.org>:
>
>> Quoting Jan Schneider <jan at horde.org>:
>>
>>> Zitat von Michael M Slusarz <slusarz at horde.org>:
>>>
>>>> Quoting Jan Schneider <jan at horde.org>:
>>>>
>>>>> commit d255d66a6a8ace592e3a058e455f628cc6aa1faa
>>>>> Author: Jan Schneider <jan at horde.org>
>>>>> Date:   Tue Jan 26 23:59:41 2010 +0100
>>>>>
>>>>> Revert "Too many htmlspecialchars in link() for title attribute"
>>>>>
>>>>> This reverts commit c92729ed228294b9d29b8a0e73be90e06ff45ab7.
>>>>>
>>>>> This broke tags that are not properly encoded in title tag  
>>>>> anymore, if being used for tooltips, e.g. in the nag portal block.
>>>>>
>>>>> framework/Core/lib/Horde.php |    5 +++--
>>>>> 1 files changed, 3 insertions(+), 2 deletions(-)
>>>>>
>>>>> http://git.horde.org/diff.php/framework/Core/lib/Horde.php?rt=horde-git&r1=13879630963e68a2bfe3f85e37082bd6d5ac6631&r2=d255d66a6a8ace592e3a058e455f628cc6aa1faa
>>>>
>>>> But now this re-breaks tags that ARE properly encoded - such as  
>>>> tooltip display of e-mail addresses in IMP (imp/mailbox.php).   
>>>> Why isn't this fixed in nag instead?
>>>
>>> Counter question: what was changed in IMP so that the tooltips no  
>>> longer work without that change?
>>> The problem that I see is, that removing that htmlspecialchars()  
>>> where it was expected to be in the past, potentially opens all  
>>> kind of XSS vulnerabilties, unless we go through every single  
>>> link() call and check if the content is properly encoded.
>>> I'm not saying that we shouldn't do this, but just changing link()  
>>> and fixing issues where we see them popping up, makes us vulnerable.
>>
>> Bringing this up from a long time ago... but this is still broken.   
>> Because it is impossible to display an ampersand in the title  
>> attribute.
>>
>> e.g. calling Horde::link('', 'A & B', ...)
>>
>> will produce a tag like the following:
>>
>> <a ... title="A &amp;amp; B" ...>
>>
>> instead of the proper "A &amp; B" title.  This is broken behavior.
>>
>> I stand by my previous fix.  At the very least, I need input on how  
>> I can get this to output correctly if I am still thought to be wrong.
>
> What about using something like escapeOnce() from Horde_View_Helper_Tag?

It seems a bit hackish... but it works.  And I am still unclear about  
why double htmlescaping the entire string is "safer" than single  
escaping, but I will trust Jan when he says this is necessary.  So  
I've added the preg regex to replace the double encoded entities to  
Horde::link().

michael

-- 
___________________________________
Michael Slusarz [slusarz at horde.org]