ZF-8828: Revise StripTags with ISO-8859-1 characters

Issue Type:
Bug
Created:
2010-01-15T05:53:01.000+0000
Last Updated:
2010-01-21T10:56:06.000+0000
Status:
Resolved
Fix version(s):
  • 1.10.0 (27/Jan/10)
Reporter:
Stephan "Bladed" de Souza (bladed)
Assignee:
Matthew Weier O'Phinney (matthew)
Tags:
  • Zend_Filter
Related issues:
Attachments:

Description

On release 20145 ( 2010-01-08 ) the comments are always striped with the follow sintax:

 
$valueCopy = preg_replace('#

Comments

Posted by Thomas Weidner (thomas) on 2010-01-15T06:13:53.000+0000

Comments are always stripped because they are potential unsecure.

To prevent code injection these are no longer allowed with the actual releases and will not be implemented again.

Posted by Matthew Weier O'Phinney (matthew) on 2010-01-15T07:13:49.000+0000

Thomas -- the problem is that non-commented strings are being stripped. I suspect this is due to using the "u" unicode flag on the regex, which isn't really needed in this particular case.

Posted by Matthew Weier O'Phinney (matthew) on 2010-01-15T07:16:45.000+0000

Please test the above patch (use "patch -p1 < strip_tags.patch" from your root directory), and let me know if it resolves the issue. I also need to run it by others on our security team.

Posted by Stephan "Bladed" de Souza (bladed) on 2010-01-15T07:36:40.000+0000

The patch works great.

Thanks.

Posted by Thomas Weidner (thomas) on 2010-01-15T08:15:08.000+0000

Matthew: I don't think that this is the real problem. I think that in Stephans environment unicode support is disabled. Because ISO-8859-1 characters are a sub-table of unicode but not reverse.

This means that we need to select between two cases: 1. Unicode enabled and 2. Unicode disabled

We need to seperate this stripping logic within 2 calls. One for unicode and one for non-unicode.

See the Alnum filter for details about what I mean.

Posted by Stephan "Bladed" de Souza (bladed) on 2010-01-15T08:30:03.000+0000

Thomas:

Tried with Alnum unicode test:


(@preg_match('/\pL/u', 'a')) ? true : false;

Windows Server w/ Apache 2.2 + PHP5.2.11: true FreeBSD w/ Apache 2.2 + PHP5.2.11: true

Posted by Thomas Weidner (thomas) on 2010-01-15T08:40:02.000+0000

Interesting because I am myself on Windows but with unicode and have no problems on ISO encoded chars.

Therefor I thought that you may have disabled unicode in your environment.

Posted by Thomas Weidner (thomas) on 2010-01-15T15:09:50.000+0000

At last I was able to reproduce this behaviour. Thanks for the patch.

Fixed with r20315

Posted by Matthew Weier O'Phinney (matthew) on 2010-01-16T08:26:03.000+0000

Thomas, please revert the patch. I'm waiting for feedback from Paddy, who reported the original security vulnerability; I don't want to re-introduce a security issue.

Posted by Matthew Weier O'Phinney (matthew) on 2010-01-16T08:27:23.000+0000

Thomas -- can you attach the patch you applied to this issue, and then revert it from the repository so we can review, please? Thanks!

Posted by Thomas Weidner (thomas) on 2010-01-16T10:41:56.000+0000

Patch attached

Reverted the changes from branch to previous buggy behaviour

Posted by Dolf Schimmel (Freeaqingme) (freak) on 2010-01-16T14:32:10.000+0000

Matthew, Paddy, as a sidenote; How about unittesting for the security testing, so s/o else won't reintroduce the issue?

Posted by Thomas Weidner (thomas) on 2010-01-16T14:44:12.000+0000

Within the patch I deleted from branch and added here as attachment are unittests contained

Posted by Matthew Weier O'Phinney (matthew) on 2010-01-21T10:56:04.000+0000

Patch applied in trunk and 1.10 release branch.