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

[Chuck Hagenbuch]

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?

-chuck