osCommerce / oscommerce2

osCommerce Online Merchant v2.x
http://www.oscommerce.com
MIT License
281 stars 222 forks source link

SQL injection vulnerability exists in oscommerce2 #672

Open ixpqxi opened 2 months ago

ixpqxi commented 2 months ago

1 Vulnerability basic information

2 Cause of vulnerability

Analysis code version: v2.3.4.1 The vulnerability code is in admin\modules.php:37,as shown below: 4 In line 36 of the code above, the program takes $key and the corresponding $value from $HTTP_POST_VARS['configuration'] in the form of key value pairs, and constructs the SQL statement with $key and $value as string concatenation in line 37.After review, the program uses do_magic_quotes_gpc() in includes\functions\compatibility.php to sanitize the $HTTP_POST_VARS variable, and the simplified code looks like this:

if (!get_magic_quotes_gpc()) {
    do_magic_quotes_gpc($HTTP_GET_VARS);
    do_magic_quotes_gpc($HTTP_POST_VARS);
    do_magic_quotes_gpc($HTTP_COOKIE_VARS);
}

function do_magic_quotes_gpc(&$ar) {
    if (!is_array($ar)) return false;

    reset($ar);
    while (list($key, $value) = each($ar)) {
      if (is_array($ar[$key])) {
        do_magic_quotes_gpc($ar[$key]);
      } else {
        $ar[$key] = addslashes($value);
      }
    }
    reset($ar);
}

A review of the above disinfection code reveals that: the program's do_magic_quotes_gpc() only sterilizes the $value of $HTTP_POST_VARS['configuration'], ignoring the value of $key, An unfiltered $key value is used to construct SQL statements in admin\modules.php:37, resulting in an SQL injection vulnerability.

3 Vulnerability recurrence

Version: v2.3.4.1

4 Vulnerability fixes

admin\modules.php:37 Add the addslashes() function to the $key argument

...
if (tep_not_null($action)) {
    switch ($action) {
      case 'save':
        reset($HTTP_POST_VARS['configuration']);
        while (list($key, $value) = each($HTTP_POST_VARS['configuration'])) {
          tep_db_query("update " . TABLE_CONFIGURATION . " set configuration_value = '" . $value . "' where configuration_key = '" . addslashes($key) . "'");
        }
...