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


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

$valueCopy = preg_replace('#


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.

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.

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.

The patch works great.


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.


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

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.

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

Fixed with r20315

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.

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

Patch attached

Reverted the changes from branch to previous buggy behaviour

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

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

Patch applied in trunk and 1.10 release branch.