Sicherheitslücke in checkout_payment_address.php oder nicht?

Informationen rund um die Sicherheit des Webs - Shops
Antworten
Xantiva
Beiträge: 948
Registriert: Mo 10. Mai 2010, 16:26
Shop Version: 1.0.10 [dev]
Kontaktdaten:

Sicherheitslücke in checkout_payment_address.php oder nicht?

Beitrag von Xantiva »

checkout_shipping_address.php:

An einigen Stellen ist in den letzten Tagen über eine mögliche Sicherheitslücke in den XT:Commerce - Forks diskutiert worden. In der checkout_shipping_address.php wurde ein $_POST Parameter ohne Prüfung in die $_SESSION übernommen:

Code: Alles auswählen

$_SESSION['sendto'] = $_POST['address'];
Auch wenn bislang noch kein Exploit dafür gezeigt werden konnte, ist es auf jeden Fall sinnvoll hier in der checkout_shipping_address.php einen sog. Typecast vorzunehmen. Also zwingend einen ganzzahligen Wert (Integer) aus der Eingabe zu machen, da vom Script her nichts anderes zu erwarten ist:

Code: Alles auswählen

$_SESSION['sendto'] = (int)$_POST['address'];
checkout_payment_address.php:

In der "verwandten" Datei checkout_payment_address.php wurde schon eine "Absicherung" der Eingabe vorgenommen:

Code: Alles auswählen

$_SESSION['billto'] = xtc_db_prepare_input($_POST['address']);
Da auch hier vom Script nichts anderes als ein Integer Wert zu erwarten ist, sollte auch in der checkout_payment_address.php der Typecast vorgenommen werden:

Code: Alles auswählen

$_SESSION['billto'] = (int)$_POST['address'];

xtc_db_prepare_input.inc.php:

Nicht zuletzt durch die Diskusssion im XTC-Modified Forum haben wir uns auch noch die Funktion xtc_db_prepare_input() angesehen. Diese arbeitet in ihrer bisherigen Version leider nicht 100%ig zuverlässig (siehe auch Sicherheitspatch von Gambio). Bisher:

Code: Alles auswählen

		$string = preg_replace('/union.*select.*from/i', '', $string);
		return trim(stripslashes($string));
Hier ist es erheblich besser die Funktion wie folgt umzustellen:

Code: Alles auswählen

		$string = stripslashes($string);
		$string = preg_replace('/union.*select.*from/i', '', $string);
		return trim($string);
Bei ersten Tests konnten wir keine Schwierigkeiten damit feststellen. Daher ist diese Anpassung ebenfalls sehr empfehlenswert!

Hier noch mal der vollständige Code der Funktion, falls der ein oder andere noch nicht den Sicherheitspatch von Gambio von November 2009 implementiert hat:

Code: Alles auswählen

function xtc_db_prepare_input($string) {
	if (is_string($string)) {
		$string = stripslashes($string);
		$string = preg_replace('/union.*select.*from/i', '', $string);
		return trim($string);
	} elseif (is_array($string)) {
		reset($string);
		while (list($key, $value) = each($string)) {
			$string[$key] = xtc_db_prepare_input($value);
		}
		return $string;
	} else {
		return $string;
	}
}
Ciao,
Mike
Mein Shop: http://www.basteln-selbermachen.de
Antworten