From c28242f63ccdd4931dc512de3f029bd995f70a33 Mon Sep 17 00:00:00 2001 From: Aleksander Machniak Date: Fri, 14 Sep 2018 13:37:19 +0200 Subject: [PATCH] Log errors caused by low pcre.backtrack_limit when sending a mail message (#6433) --- CHANGELOG | 1 + INSTALL | 1 + program/lib/Roundcube/rcube_utils.php | 37 +++++++++++++++++++++++++ program/lib/Roundcube/rcube_washtml.php | 23 ++++----------- program/steps/mail/sendmail.inc | 12 ++++++++ 5 files changed, 56 insertions(+), 18 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 233b4b898..e9a95e0a6 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -9,6 +9,7 @@ CHANGELOG Roundcube Webmail - Password: Fix bug where password_dovecotpw_with_method setting could be ignored (#6436) - Elastic: Improved UX of search dialogs (#6416) - Elastic: Fix unwanted thread expanding when selecting a collapsed thread in non-mobile mode (#6445) +- Log errors caused by low pcre.backtrack_limit when sending a mail message (#6433) - Fix so invalid smtp_helo_host is never used, fallback to localhost (#6408) - Fix custom logo size in Elastic (#6424) - Fix listing the same attachment multiple times on forwarded messages diff --git a/INSTALL b/INSTALL index aab83e65f..9c62bbb14 100644 --- a/INSTALL +++ b/INSTALL @@ -33,6 +33,7 @@ REQUIREMENTS - session.auto_start disabled - suhosin.session.encrypt disabled - mbstring.func_overload disabled + - pcre.backtrack_limit >= 100000 * A MySQL, PostgreSQL, MS SQL Server (2005 or newer), Oracle database or SQLite support in PHP - with permission to create tables * Composer installed either locally or globally (https://getcomposer.org) diff --git a/program/lib/Roundcube/rcube_utils.php b/program/lib/Roundcube/rcube_utils.php index 880b9c259..decc015c4 100644 --- a/program/lib/Roundcube/rcube_utils.php +++ b/program/lib/Roundcube/rcube_utils.php @@ -1350,4 +1350,41 @@ class rcube_utils return $max_filesize; } + + /** + * Detect and log last PREG operation error + * + * @param array $error Error data (line, file, code, message) + * @param bool $terminate Stop script execution + * + * @return bool True on error, False otherwise + */ + public static function preg_error($error = array(), $terminate = false) + { + if (($preg_error = preg_last_error()) != PREG_NO_ERROR) { + $errstr = "PCRE Error: $preg_error."; + + if ($preg_error == PREG_BACKTRACK_LIMIT_ERROR) { + $errstr .= " Consider raising pcre.backtrack_limit!"; + } + if ($preg_error == PREG_RECURSION_LIMIT_ERROR) { + $errstr .= " Consider raising pcre.recursion_limit!"; + } + + $error = array_merge(array('code' => 620, 'line' => __LINE__, 'file' => __FILE__), $error); + + if (!empty($error['message'])) { + $error['message'] .= ' ' . $errstr; + } + else { + $error['message'] = $errstr; + } + + rcube::raise_error($error, true, $terminate); + + return true; + } + + return false; + } } diff --git a/program/lib/Roundcube/rcube_washtml.php b/program/lib/Roundcube/rcube_washtml.php index 856027265..4c3d112bb 100644 --- a/program/lib/Roundcube/rcube_washtml.php +++ b/program/lib/Roundcube/rcube_washtml.php @@ -617,6 +617,11 @@ class rcube_washtml $html = preg_replace($html_search, $html_replace, trim($html)); + $err = array('line' => __LINE__, 'file' => __FILE__, 'message' => "Could not clean up HTML!"); + if ($html === null && rcube_utils::preg_error($err)) { + return ''; + } + // Replace all of those weird MS Word quotes and other high characters $badwordchars = array( "\xe2\x80\x98", // left single quote @@ -638,24 +643,6 @@ class rcube_washtml $html = str_replace($badwordchars, $fixedwordchars, $html); - // PCRE errors handling (#1486856), should we use something like for every preg_* use? - if ($html === null && ($preg_error = preg_last_error()) != PREG_NO_ERROR) { - $errstr = "Could not clean up HTML message! PCRE Error: $preg_error."; - - if ($preg_error == PREG_BACKTRACK_LIMIT_ERROR) { - $errstr .= " Consider raising pcre.backtrack_limit!"; - } - if ($preg_error == PREG_RECURSION_LIMIT_ERROR) { - $errstr .= " Consider raising pcre.recursion_limit!"; - } - - rcube::raise_error(array('code' => 620, 'type' => 'php', - 'line' => __LINE__, 'file' => __FILE__, - 'message' => $errstr), true, false); - - return ''; - } - // fix (unknown/malformed) HTML tags before "wash" $html = preg_replace_callback('/(<(?!\!)[\/]*)([^\s>]+)([^>]*)/', array($this, 'html_tag_callback'), $html); diff --git a/program/steps/mail/sendmail.inc b/program/steps/mail/sendmail.inc index 69dcca43c..f85b8fcb6 100644 --- a/program/steps/mail/sendmail.inc +++ b/program/steps/mail/sendmail.inc @@ -115,6 +115,12 @@ if (!$savedraft) { '


', ), $message_body); + + rcube_utils::preg_error(array( + 'line' => __LINE__, + 'file' => __FILE__, + 'message' => "Could not format HTML!" + ), true); } // Check spelling before send @@ -199,6 +205,12 @@ if (is_array($COMPOSE['attachments'])) { $message_body = preg_replace($dispurl, '"cid:' . $cid . '"', $message_body); + rcube_utils::preg_error(array( + 'line' => __LINE__, + 'file' => __FILE__, + 'message' => "Could not replace an image reference!" + ), true); + $MAIL_MIME->setHTMLBody($message_body); if ($attachment['data'])