Remove create_function calls, modcss.php, and clean up HTML formatting code

[jeblair]

The following patch cleans up some code that is no longer relevant, and fixes some potential security problems related to the disused code.

The PHP security framework Suhosin allows administrators to blacklist certain dangerous PHP functions such as create_function. If RoundCube doesn't need to use create_function, then the removal of those calls will allow admins to keep create_function blacklisted, improving site security.

We recently audited RoundCube's use of PHP's create_function call, and found that in the few times it is used it can be easily avoided, or in one case removed entirely. create_function with preg_replace_callback is suggested as an alternative to the '/e' syntax of preg_replace, but without considerable care, can be just as easily exploited. See this article for more information:

http://www.securityfocus.com/bid/31398/info

Over time, HTML sanitizing has changed significantly in RoundCube, and the code around rcmail_mod_css_styles and its uses of create_function has particularly been affected. The patch does the following:

1) Removes the preg_replace_callback that handles @import statements in rcmail_mod_css_styles.

This preg_replace will never actually run because rcmail_mod_css_styles now (as of changeset [1773]) refuses to handle @import statements at all (see the block at the start of the function labeled "ignore the whole block if evil styles are detected".

Note that this use of create_function is theoretically exploitable, because of the way $base_url is used. However, $base_url can no longer be set because the HTML washer does not allow the <base> tag, so this is not currently exploitable.

2) Change the <<str_replacement>> preg_replace_callback in preg_replace_callback to use a helper class to keep track of the CSS string replacements.

This lets us get rid of a create_function call and is a little cleaner.

3) Change the preg_replace_callback in rcmail_xss_entitiy_decode to use a helper callback instead of create_function.

4) Remove references to $base_url in rcmail_htm4inline.

The HTML washer does not allow the <base> tag, so this can never be set in practice. Removing it simplifies rcmail_htm4inline considerably.

5) Stop handling the <link> tag in rcmail_alter_html_link, only handle the <a> tag.

The <link> tag is not allowed by the washer, and so this code is no longer relevant.

Finally, after these changes, the entire file bin/modcss.php can be removed as there are no longer any references to it in the source code. The previous references were removed by changes (1) and (5).

[thomasb]

You're right that @import statements will never occur in CSS anymore and string replacements could be implemented without using create_function.

But the fact that washtml removes <base> and <link> tags is actually a bug and doesn't mean that we can remove this functionality entirely. modcss.php will therefore not become obsolete because we still need it for <link> tags.

We'll try to get the best out of your patch.

[thomasb]

Fixed in r2252