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

[Michael M Slusarz]

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.

The previous fix is reproduced here

=====

commit c92729ed228294b9d29b8a0e73be90e06ff45ab7
Author: Michael M Slusarz <slusarz at curecanti.org>
Date:   Wed Jan 20 12:16:21 2010 -0700

     Too many htmlspecialchars in link() for title attribute

     For example, things like '<' should be specified as '&lt;', NOT
     '&amp;&lt;'

diff --git a/framework/Core/lib/Horde.php b/framework/Core/lib/Horde.php
index f764f8b..16aa953 100644
--- a/framework/Core/lib/Horde.php
+++ b/framework/Core/lib/Horde.php
@@ -1293,9 +1293,8 @@ HTML;
                  $old_error = error_reporting(0);
                  $title = str_replace(
                      array("\r", "\n"), '',
-                    htmlspecialchars(
-                        nl2br(htmlspecialchars($title, ENT_QUOTES,  
$charset)),
-                        ENT_QUOTES, $charset));
+                    nl2br(htmlspecialchars($title, ENT_QUOTES, $charset))
+                );
                  error_reporting($old_error);
              }
              $attributes['title.raw'] = $title;

=====

michael

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