Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#1485689 closed Bugs (fixed)

use preg_replace_callback instead of preg_replace

Reported by: paul Owned by:
Priority: 5 Milestone: 0.2.1
Component: Addressbook Version: git-master
Severity: normal Keywords:
Cc: volker@…

Description

Hi,

As you know, there has been a vulnerability in roundcube 0.1 and 0.2 beta (see ticket #1485618), caused by using preg_replace_match with the 'e' flag. To rule out any more vulnerabilities, please consider using preg_replace_callback instead. In particular, the function rfc2425_fold in rcube_vcard.php looks dubious. Please apply this patch.

Attachments (1)

rcube_vcard.diff (610 bytes) - added by paul 6 years ago.

Download all attachments as: .zip

Change History (10)

Changed 6 years ago by paul

comment:1 Changed 6 years ago by paul

  • Summary changed from use preg_replace_callback instead of preg_replace_match to use preg_replace_callback instead of preg_replace

comment:2 Changed 6 years ago by alec

  • Milestone changed from later to 0.2.1

comment:3 Changed 6 years ago by alec

  • Component changed from Addressbook to Security issue
  • Resolution set to fixed
  • Status changed from new to closed

Fixed in [478c7c63].

comment:4 Changed 6 years ago by volker

  • Cc volker@… added
  • Component changed from Security issue to Addressbook
  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Type changed from Patches to Bugs

Hi Alec,

I think I found a bug in your fix. I tried to import some vCards which I got from my Apple Addressbook. First tries stopped and produced the following fatal error:

[08-Feb-2009 18:29:03] PHP Warning:  include_once(self.php) [<a href='function.include-once'>function.include-once</a>]:
      failed to open stream: No such file or directory in /usr/share/psa-horde/program/include/iniset.php on line 98
[08-Feb-2009 18:29:03] PHP Warning:  include_once() [<a href='function.include'>function.include</a>]:
      Failed opening 'self.php' for inclusion (include_path='/usr/share/psa-horde/:/usr/share/psa-horde/program:
      /usr/share/psa-horde/program/lib:/usr/share/psa-horde/program/include:/usr/share/psa-horde/lib:/usr/share/psa-pear:.')
      in /usr/share/psa-horde/program/include/iniset.php on line 98
[08-Feb-2009 18:29:03] PHP Fatal error:  Cannot call method self::rfc2425_fold_callback() or method does not exist in
      /usr/share/psa-horde/program/include/rcube_vcard.php on line 236

If I remove the static keyword the import finishes but with warnings.

[08-Feb-2009 18:36:32] PHP Warning:  preg_replace_callback() [<a href='function.preg-replace-callback'>function.preg-replace-callback</a>]:
      Requires argument 2, 'rfc2425_fold_callback', to be a valid callback in /usr/share/psa-horde/program/include/rcube_vcard.php on line 236

How can I fix this? I'm using the lastest trunk and I'm updating twice a week.

comment:5 Changed 6 years ago by volker

  • Version changed from 0.2-stable to svn-trunk

comment:6 follow-ups: Changed 6 years ago by thomasb

  • Resolution set to fixed
  • Status changed from reopened to closed

The current RoundCube version requires PHP5. Works fine for me.

comment:7 in reply to: ↑ 6 Changed 6 years ago by volker

Replying to thomasb:

The current RoundCube version requires PHP5. Works fine for me.

Hi Thomas,

I'm using PHP 5.2.4-2ubuntu5.4 with Suhosin-Patch 0.9.6.2. But thanks for the hint. I set line 236 in rcube_vcard.php from:

return preg_replace_callback('/:([^\n]{72,})/', 'self::rfc2425_fold_callback', $val) . "\n";

to:

return preg_replace_callback('/:([^\n]{72,})/', 'rcube_vcard::rfc2425_fold_callback', $val) . "\n";

Now it runs without any warning or error and works fine for me too.

Maybe the failure occurred because I have open_basedir restriction activated, but I don't know 100%.

Thanks for your great work so far. I'm looking forward to all the new features you'll develop.

comment:8 in reply to: ↑ 6 Changed 6 years ago by volker

Replying to thomasb:

The current RoundCube version requires PHP5. Works fine for me.

And the "correct" way using a static class function in PHP5 as callback is:

preg_replace_callback(pattern, array('self', 'methodName'), subject)

As seen on: http://algorytmy.pl/doc/php/function.preg-replace-callback.php at the bottom of the page.

comment:9 Changed 6 years ago by alec

And more here http://pl.php.net/manual/pl/language.pseudo-types.php#language.types.callback . Passing callbacks as array is safer. Fixed in [5e68157c].

Note: See TracTickets for help on using tickets.