A vulnerability in WebEdition CMS's handling of INSERT/UPDATE SQL queries allows SQL injection.

Introduction WebEdition CMS is an open source CMS written in PHP that seems to be mostly used by german websites. It came to our attention a few months ago, because another party performed an audit on it and came up with some vulnerabilities. Because we always look for nice PHP bugs for our own PHP and web security trainings we had a very quick look into it and were able to find a number of vulnerabilites that we disclosed to the vendor. The most serious vulnerability that we discovered was a remote PHP code execution vulnerability that we discussed in a previous blog entry. Because the disclose process took a while for the previous vulnerability we accidentally spend a bit more time in the code and realized a vulnerability in their handling of UPDATE and INSERT by some helper functions they use. This vulnerability allows for SQL injections into UPDATE/INSERT SQL statements that make use of these helper functions. The vendor incorporated a fix for this vulnerability into the release of WebEdition 6.3.8-s2. Unfortunately the vendor was very hard to work with and they decided to not ask us for help or review of their fixes. Therefore as you will learn at the end of this blog posting their fix is incomplete. As mentioned in our previous blog posting the only way to get this security patch is to use their OnlineInstaller. They still do not have a tarball for you to download. We strictly advise against shipping PHP software in this way, because it requires the document root and all its subdirectories to be writable, which is a very bad idea in terms of security. In this post we will discuss the SQL injection vulnerability in WebEdition's UPDATE/INSERT helper functions and show how they fixed the problem and discuss why this is incomplete.

The Vulnerability When you look into how WebEdition handled SQL queries you will sooner or later see that they use some helper functions to handle UPDATE and INSERT statements, if multiple database fields require updating. Here is one example SQL query from bannerclick.php: <?php $db -> query ( 'INSERT INTO ' . BANNER_CLICKS_TABLE . ' SET ' . we_database_base :: arraySetter ( array ( 'ID' => intval ( $id ), 'Timestamp' => sql_function ( 'UNIX_TIMESTAMP()' ), 'IP' => $_SERVER [ 'REMOTE_ADDR' ], 'Referer' => ( $referer ? $referer : ( isset ( $_SERVER [ 'HTTP_REFERER' ]) ? $_SERVER [ 'HTTP_REFERER' ] : '' )), 'DID' => intval ( $did ), 'Page' => $page ))); As you can see they use a helper function called we_database_base::arraySetter to convert an array of values into a comma separated list of SQL assignments. The code for this can be found in /we/include/we_classes/database/we_database_base.class.php: <?php abstract class we_database_base { static function arraySetter ( array $arr , $imp = ',' ){ $ret = array (); foreach ( $arr as $key => $val ){ $escape = ! ( is_int ( $val ) || is_float ( $val )); if ( is_array ( $val ) && $val [ 'sqlFunction' ] == 1 ){ $val = $val [ 'val' ]; $escape = false ; } elseif ( is_object ( $val ) || is_array ( $val )){ t_e ( 'warning' , 'data error: db-field cannot contain objects / arrays' , 'Key: ' . $key , $arr ); } ... $ret [] = '`' . $key . '`=' . ( $escape ? '"' . escape_sql_query ( $val ) . '"' : $val ); } return implode ( $imp , $ret ); } In this code you can see that they use a database escaping function escape_sql_query() to prepare values before they are inserted in between double quotes inside the SQL statement. For completness lets have a look into the escape_sql_query() function although it is not involved in the SQL injection problem we reported. The function can be found in /we/include/we_db_tools.inc.php. <?php function escape_sql_query ( $inp ){ if ( is_array ( $inp )){ return array_map ( __METHOD__ , $inp ); } if ( ! empty ( $inp ) && is_string ( $inp )){ return str_replace ( array ( '\\' , " \0 " , "

" , " \r " , "'" , '"' , " \x1a " ), array ( '\\\\' , '\\0' , '\

' , '\\r' , " \\ '" , '\\"' , '\\Z' ), $inp ); } return $inp ; } First of all this function might do the job for western character single byte character sets or even in case of UTF-8, but it is dangerous the moment the database runs with another multibyte character set. Because of this you should always use database specific escaping functions that are aware of the current character set of the database. But lets ignore that for now, because for the mostly german installations this is not really a problem. The problem is located inside the we_database_base::arraySetter() method itself. Take a look at the following lines of code and let them sink in a bit. <?php $escape = ! ( is_int ( $val ) || is_float ( $val )); if ( is_array ( $val ) && $val [ 'sqlFunction' ] == 1 ){ $val = $val [ 'val' ]; $escape = false ; } ... {} ... $ret [] = '`' . $key . '`=' . ( $escape ? '"' . escape_sql_query ( $val ) . '"' : $val ); This code has some hardcoded special cases that do not get escaped at all. The exceptions are integer values, floating point values and caller submitted SQL function invocations. Now look at the code again how do they detect a caller submitted sqlFunction? They detect it by seeing that the value provided is actually and array and contains the key sqlFunction which is set to 1. This should raise an alarm in your head, but if not remember that the values provided are actually from user input. What if that user input is an array instead of a string, what if the array contains a key sqlFunction that is set to 1 and some malicious data in a key called val? Should that ever happen then the code will use the malicious value from the val key and do not escape it at all, which will allow SQL injection. Now before getting too excited we have to check if this can ever happen and therefore we need to backtrack some calls to arraySetter and evaluate the input parameters. While we_database_base::arraySetter() is called a number of times we can find some problematic case easily inside getBanner.php. But this is not the only place. <?php ... $page = isset ( $_GET [ "page" ]) ? $_GET [ "page" ] : "" ; ... $GLOBALS [ 'DB_WE' ] -> query ( 'INSERT INTO ' . BANNER_VIEWS_TABLE . ' SET ' . we_database_base :: arraySetter ( array ( 'ID' => intval ( $id ), 'Timestamp' => sql_function ( 'UNIX_TIMESTAMP()' ), 'IP' => $_SERVER [ 'REMOTE_ADDR' ], 'Referer' => ( $referer ? $referer : ( isset ( $_SERVER [ 'HTTP_REFERER' ]) ? $_SERVER [ 'HTTP_REFERER' ] : '' )), 'DID' => intval ( $did ), 'Page' => $page ))); As you can see the $page variable is populated directly from user input inside the URL $_GET["page"] and used in the call to we_database_base::arraySetter(). This means if our URL contains the following it will result in SQL injection. .../getBanner.php?...&page[val]=MALICIOUS_SQL_INJECTION&page[sqlFunction]=1&... With a bit of patience you could now evaluate every single call to we_database_base::arraySetter() and find some other potential entrypoint for this SQL injection problem. We leave this excersise to the reader. Well the correct way to fix this problem is to fix the we_database_base::arraySetter() anyway, which is excatly what the vendor attempted. Unfortunately they partially failed.