Lücke in WordPress ermöglicht Aussperren des Admins

2009-08-11

Gerade eben gelesen (thx to Spreeblick): Lücke in WordPress ermöglicht Aussperren des Admins.

Also nichts wie los liebe WordPress-Admins, schnell die wp-login.php ändern.

Genaue Infos zum Bug gibt’s von Laurent Gaffié. Der Bug wird aber nicht als kritisch eingestuft, da es einem Angreifer keinen Vorteil verschafft, aber es führt zu ein bisschen Ärger beim Admin.

Der Bug ist übrigens ein sehr gutes Beispiel für unsaubere Programmierung. Ein Eingabeparameter wird lediglich mittels empty($key) überprüft. Sinnvoller als zu überprüfen wie ein Parameter nicht beschaffen sein soll, ist es immer zu überprüfen WIE genau der Inhalt der Variable aussehen muss.

Also zum Beispiel könnte man an dieser Stelle eine Stringconversion und nachträglich eine Überprüfung auf Mindestlänge und enthaltene Zeichen durchführen:

$key = (string) $key;
if (!preg_match("~[A-Za-z0-9]{20}~", $key) ) {
return new WP_Error('invalid_key', __('Invalid key'));
}

Das ganze jetzt so aus der Hand geschrieben, aber sollte so in etwa funktionieren. Unter der Bedingung, dass

$key

immer 20 Zeichen lang ist, was ich jetzt nicht weiß.

Eine Zeile weiter oben heißt es sogar schon:

$key = preg_replace('/[^a-z0-9]/i', '', $key);

Allerdings ohne vorher auf einen String oder die Länge zu überprüfen.

Achja, ist so ein ganz klarer “Mit Java wäre das nicht passiert”-Bug ;-)

Update:

Inzwischen ist das Problem gefixt. Und zwar so:

190 if ( empty( $key ) || !is_string( $key ) )
191 return new WP_Error(‘invalid_key’, __(‘Invalid key’));
192
193 if ( empty($login) || !is_string($login) )
194 return new WP_Error(‘invalid_key’, __(‘Invalid key’));
195
196 $user = $wpdb->get_row($wpdb->prepare(“SELECT * FROM $wpdb->users WHERE user_activation_key = %s AND user_login = %s”, $key, $login));
197 if ( empty( $user ) )
198 return new WP_Error(‘invalid_key’, __(‘Invalid key’));

Das ist auch ok. Überprüfung ob es ein String ist. Auf jeden Fall besser als zu überprüfen ob es kein Array ist.