From 037af6890fe6fdb84a08d3c86083e847c90ec0ad Mon Sep 17 00:00:00 2001 From: Aleksander Machniak <alec@alec.pl> Date: Tue, 22 Oct 2013 08:17:26 -0400 Subject: [PATCH] Fix vulnerability in handling _session argument of utils/save-prefs (#1489382) --- program/lib/Roundcube/rcube_db.php | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++------- 1 files changed, 101 insertions(+), 15 deletions(-) diff --git a/program/lib/Roundcube/rcube_db.php b/program/lib/Roundcube/rcube_db.php index 47ddc81..b3f549b 100644 --- a/program/lib/Roundcube/rcube_db.php +++ b/program/lib/Roundcube/rcube_db.php @@ -70,7 +70,7 @@ $driver = isset($driver_map[$driver]) ? $driver_map[$driver] : $driver; $class = "rcube_db_$driver"; - if (!class_exists($class)) { + if (!$driver || !class_exists($class)) { rcube::raise_error(array('code' => 600, 'type' => 'db', 'line' => __LINE__, 'file' => __FILE__, 'message' => "Configuration error. Unsupported database driver: $driver"), @@ -128,7 +128,7 @@ $dsn_string = $this->dsn_string($dsn); $dsn_options = $this->dsn_options($dsn); - if ($db_pconn) { + if ($this->db_pconn) { $dsn_options[PDO::ATTR_PERSISTENT] = true; } @@ -400,21 +400,27 @@ $this->debug($query); - $query = $this->dbh->query($query); + // destroy reference to previous result, required for SQLite driver (#1488874) + $this->last_result = null; + $this->db_error_msg = null; - if ($query === false) { + // send query + $result = $this->dbh->query($query); + + if ($result === false) { $error = $this->dbh->errorInfo(); $this->db_error = true; $this->db_error_msg = sprintf('[%s] %s', $error[1], $error[2]); rcube::raise_error(array('code' => 500, 'type' => 'db', 'line' => __LINE__, 'file' => __FILE__, - 'message' => $this->db_error_msg), true, false); + 'message' => $this->db_error_msg . " (SQL Query: $query)" + ), true, false); } - $this->last_result = $query; + $this->last_result = $result; - return $query; + return $result; } /** @@ -422,7 +428,7 @@ * * @param mixed $result Optional query handle * - * @return int Number of rows or false on failure + * @return int Number of (matching) rows */ public function affected_rows($result = null) { @@ -431,6 +437,32 @@ } return 0; + } + + /** + * Get number of rows for a SQL query + * If no query handle is specified, the last query will be taken as reference + * + * @param mixed $result Optional query handle + * @return mixed Number of rows or false on failure + * @deprecated This method shows very poor performance and should be avoided. + */ + public function num_rows($result = null) + { + if ($result || ($result === null && ($result = $this->last_result))) { + // repeat query with SELECT COUNT(*) ... + if (preg_match('/^SELECT\s+(?:ALL\s+|DISTINCT\s+)?(?:.*?)\s+FROM\s+(.*)$/ims', $result->queryString, $m)) { + $query = $this->dbh->query('SELECT COUNT(*) FROM ' . $m[1], PDO::FETCH_NUM); + return $query ? intval($query->fetchColumn(0)) : false; + } + else { + $num = count($result->fetchAll()); + $result->execute(); // re-execute query because there's no seek(0) + return $num; + } + } + + return false; } /** @@ -566,7 +598,7 @@ * Formats input so it can be safely used in a query * * @param mixed $input Value to quote - * @param string $type Type of data + * @param string $type Type of data (integer, bool, ident) * * @return string Quoted/converted string for use in query */ @@ -579,6 +611,10 @@ if (is_null($input)) { return 'NULL'; + } + + if ($type == 'ident') { + return $this->quote_identifier($input); } // create DB handle if not available @@ -599,6 +635,22 @@ } /** + * Escapes a string so it can be safely used in a query + * + * @param string $str A string to escape + * + * @return string Escaped string for use in a query + */ + public function escape($str) + { + if (is_null($str)) { + return 'NULL'; + } + + return substr($this->quote($str), 1, -1); + } + + /** * Quotes a string so it can be safely used as a table or column name * * @param string $str Value to quote @@ -610,6 +662,20 @@ public function quoteIdentifier($str) { return $this->quote_identifier($str); + } + + /** + * Escapes a string so it can be safely used in a query + * + * @param string $str A string to escape + * + * @return string Escaped string for use in a query + * @deprecated Replaced by rcube_db::escape + * @see rcube_db::escape + */ + public function escapeSimple($str) + { + return $this->escape($str); } /** @@ -630,7 +696,7 @@ $name[] = $start . $elem . $end; } - return implode($name, '.'); + return implode($name, '.'); } /** @@ -647,7 +713,7 @@ * Return list of elements for use with SQL's IN clause * * @param array $arr Input array - * @param string $type Type of data + * @param string $type Type of data (integer, bool, ident) * * @return string Comma-separated list of quoted values for use in query */ @@ -723,12 +789,19 @@ /** * Encodes non-UTF-8 characters in string/array/object (recursive) * - * @param mixed $input Data to fix + * @param mixed $input Data to fix + * @param bool $serialized Enable serialization * * @return mixed Properly UTF-8 encoded data */ - public static function encode($input) + public static function encode($input, $serialized = false) { + // use Base64 encoding to workaround issues with invalid + // or null characters in serialized string (#1489142) + if ($serialized) { + return base64_encode(serialize($input)); + } + if (is_object($input)) { foreach (get_object_vars($input) as $idx => $value) { $input->$idx = self::encode($value); @@ -739,6 +812,7 @@ foreach ($input as $idx => $value) { $input[$idx] = self::encode($value); } + return $input; } @@ -748,12 +822,24 @@ /** * Decodes encoded UTF-8 string/object/array (recursive) * - * @param mixed $input Input data + * @param mixed $input Input data + * @param bool $serialized Enable serialization * * @return mixed Decoded data */ - public static function decode($input) + public static function decode($input, $serialized = false) { + // use Base64 encoding to workaround issues with invalid + // or null characters in serialized string (#1489142) + if ($serialized) { + // Keep backward compatybility where base64 wasn't used + if (strpos(substr($input, 0, 16), ':') !== false) { + return self::decode(@unserialize($input)); + } + + return @unserialize(base64_decode($input)); + } + if (is_object($input)) { foreach (get_object_vars($input) as $idx => $value) { $input->$idx = self::decode($value); -- Gitblit v1.9.1