Investigation AppSec : une vulnérabilité critique sur un plugin Wordpress
Voici un premier type d'article d'une longue liste ! Nous lançons nos AppSec Investigation. L'idée n'est pas de faire une démonstration de l'attaque. Ce genre de présentation est souvent omniprésente sur Internet et sur les réseaux sociaux comme Linkedin.
Nous aimerions vous montrer un autre angle : un point de vue AppSec, le point de vue de la sécurité applicative.
Au lieu de faire un simple résumé d’une vulnérabilité ou de vous noyer sous des chiffres, nous vous emmènerons dans les coulisses de vraie investigation AppSec – façon “live coding” ou “live Twitch” où on décortique tout ensemble. Nous allons plonger dans le code pour comprendre pourquoi et comment c’était vulnérable, et de voir comment les développeurs ont corrigé le problème.
[1/4] Alors si vous êtes prêt, commencons notre première investigation !
La vulnérabilité que je vais analyser aujourd'hui est la CVE-2024-10924 concernant le plugin WordPress Really Simple Security.
Cette vulnérabilité a été taggé comme critique ! Pourquoi ? Parce qu'elle rendait presque 4 000 000 (millions) de sites WordPress vulnérables à une vulnérabilité de type Authentication Bypass. Pour faire simple, sous certaines conditions, n'importe qui pouvait avoir un accès au panel d'administration du site.
Avant d’analyser le code, regardons à quoi sert ce plugin Really Simple Security.
D’après sa fiche officielle, il aide à configurer facilement plusieurs procédures de sécurité : activer le SSL automatiquement, désactiver des features risquées, améliorer la config, etc.
Nous, ce qui nous intéresse, c’est la fonctionnalité “2FA”. Pourquoi ? Parce que d’après la CVE, c'est dans cette fonctionnalité que la vulnérabilité d’Authentication Bypass. Aie, c’est un comble pour une fonctionnalité de MFA !
Bon du coup, qu’est ce dit la CVE ?
"This is due to improper user check error handling in the two-factor REST API actions with the 'check_login_and_get_user' function. This makes it possible for unauthenticated attackers to log in as any existing user on the site, such as an administrator, when the "Two-Factor Authentication" setting is enabled (disabled by default)."
On comprends que le plugin dispose de sa propre API, et une fonction check_login_and_get_user() qui à l'air de ne pas faire son job correctement.
Allons voir ça de plus prêt ! Ouvrons le fichier codeguardian-lab/wp-content/plugins/really-simple-ssl/security/wordpress/two-fa/class-rsssl-two-factor-on-board-api.php
Et, oui ! Effectivement on y voit plusieurs déclarations de nouveau endpoint. On en compte 7 liés à la fonctionnalité 2FA :
- /save_default_method_email - choix du mode OTP ?
- /resend_email_code - pour renvoyer un code pour les gens non-rapide
- /skip_onboarding - un skip_on..? Pardon ? Ahh, d’après le code, il s’agit d’une fonctionnalité pour prévenir l’utilisateur qu’il faut qu’il définisse son 2FA sous X jours. Pendant ces X jours, il peut skip l’authentification 2FA. Une sorte de période d'acceptation 🙃
- etc.
Exemples :
register_rest_route(
self::NAMESPACE,
'do_not_ask_again',
array(
'methods' => 'POST',
'callback' => array( $this, 'disable_two_fa_for_user' ),
'permission_callback' => function ( WP_REST_Request $request ) {
return true; // Allow all requests; handle auth in the callback.
},
'args' => array(
'redirect_to' => array(
'required' => false,
'type' => 'string',
),
'user_id' => array(
'required' => true,
'type' => 'integer',
),
'login_nonce' => array(
'required' => true,
'type' => 'string',
),
),
)
);
register_rest_route(
self::NAMESPACE,
'skip_onboarding',
array(
'methods' => 'POST',
'callback' => array( $this, 'skip_onboarding' ),
'permission_callback' => '__return_true',
'args' => array(
'redirect_to' => array(
'required' => false,
'type' => 'string',
),
'user_id' => array(
'required' => true,
'type' => 'integer',
),
'login_nonce' => array(
'required' => true,
'type' => 'string',
),
),
)
);
Rien d’étonnant à ce que skip_onboarding ait attiré l’attention dans les writeups ou le bug bounty : elle donne envie de gratter un peu plus …
Bon … Déjà, est-ce que vous aussi vous avez vu quelque chose qui vous pique un peu vos yeux d’expert sur ces déclarations d'endpoint ?
Regardez la déclaration des endpoints. La permission_callback renvoie systématiquement true. On s’attends à voir une vérification d’authentification ici, et rien…
Après réflexion, on pourrait ce dire que c’est normal. Ça peut se justifier en partie : l’utilisateur n’est pas encore totalement logué puisqu’on est en plein milieu du processus 2FA.
Mais côté DX (Developer eXperience), c’est confus : on ne sait pas si un minimum de contrôle existe du coup. Mauvaise pratique 👎.
Autre petit détail : pour la plupart des endpoints, on a :
'permission_callback' => function ( WP_REST_Request $request ) {
return true;
}
Mais pour l’endpoint skip_onboarding, on a uniquement :
'permission_callback' => '__return_true'
Ça sent l’endpoint codé à part, ou à une autre période… Bref, intriguant.
Je dirais même que c’est à la limite de la provocation ! 😈
[2/4] Assez blablaté, analysons l'endpoint skip_onboarding
/**
* Skips the onboarding process for the user.
*
* @param WP_REST_Request $request The REST request object.
*
* @return WP_REST_Response The REST response object.
*/
public function skip_onboarding( WP_REST_Request $request ): WP_REST_Response {
$parameters = new Rsssl_Request_Parameters( $request );
// As a double we check the user_id with the login nonce.
$user = $this->check_login_and_get_user( (int)$parameters->user_id, $parameters->login_nonce );
return $this->authenticate_and_redirect( $parameters->user_id, $parameters->redirect_to );
}
Bon c’est direct. Elle fait trois choses :
- Utilise une classe pour extraire les paramètres de la request.
- Appelle check_login_and_get_user() pour valider le login_nonce et retourner l’utilisateur.
- Lance la procédure d’authentification + redirection
Si on regarde les autres endpoints, ils sont tous codé de la même façon.
- Extraction des paramètres
- Appel à check_login_and_get_user()
- Use case
Ah ! On a une réponse à ma frustration de tout à l’heure ! Concernant les fonctions de callback des endpoints permission_callback. Les développeurs ont préféré mettre la vérification dans chaque endpoint unitairement que dans la déclaration des endpoints.
Encore une fois, d’un point de vue DX, j’aurais préféré voir l’appel de cette fonction lors de la déclaration de l’endpoint, et pas dans chaque méthode.
[3/4] Bon, je crois qu’il est temps d’aller au bout !
Ouvrons la fonction check_login_and_get_user()
/**
* Verifies a login nonce, gets user by the user id, and returns an error response if any steps fail.
*
* @param int $user_id The user ID.
* @param string $login_nonce The login nonce.
*
* @return WP_User|WP_REST_Response
*/
private function check_login_and_get_user( int $user_id, string $login_nonce ) {
if ( ! Rsssl_Two_Fa_Authentication::verify_login_nonce( $user_id, $login_nonce ) ) {
return new WP_REST_Response( array( 'error' => 'Invalid login nonce' ), 403 );
}
/**
* Get the user by the user ID.
*
* @var WP_User $user
*/
$user = get_user_by( 'id', $user_id );
return $user;
}
Essayons de décortiquer ça ! On vérifie si le nonce est valide et qu’il est correspond à l’utilisateur qui essaie de s’authentifier.
- Si valide ✅, pas de problème, on va chercher l’utilisateur en base de données et on le retourne
- Si non ❌, on retourne un objet de type WP_REST_Response
Et bien la voilà, on l’a notre vulnérabilité ! C’est ici que ça coince.
Problème : si le nonce n’est pas valide, on retourne juste un WP_REST_Response … mais la fonction ne se coupe pas.
Du coup, dans la suite, on appelle quand même la fonction d’authentification… avec les infos de la requête !
Certes le contrôle du login_nonce est fait, mais le script ne s’arrête pas ! Si on revient sur la fonction précédente, on ne vérifie pas si le user ≠ null OU si c’est une WP_REST_Response. Donc que le nounce soit valide ou non, peu importe ! Le script appellera la fonction d’authentification. Et en plus, on y passe comme argument directement les valeurs venant de la requête 👎.
Petit schéma récapitulatif :
public function skip_onboarding( WP_REST_Request $request ): WP_REST_Response {
$parameters = new Rsssl_Request_Parameters( $request );
$user = $this->check_login_and_get_user( (int)$parameters->user_id, $parameters->login_nonce );
// ICI on ne controle pas la variable $user
// C'est soit un WP_User soit une WP_Response si echec
// L'instruction d'après utilise $parameters qui n'est jamais validé non plus
// On récupère donc toujours un cookie de l'utilisateur passé en paramètre
return $this->authenticate_and_redirect( $parameters->user_id, $parameters->redirect_to );
}
Si je résume les problèmes que je trouve embêtant et qui entoure la vulnérabilité :
- Ambiguïté sur le contrôle des permissions lors de la création des endpoints
- Utilisation des données venant de la request sans validation
- N’échoue pas correctement sur la vérification du login_nonce
[4/4] Maintenant, voyons comment les développeurs ont corrigé la vulnérabilité !
public function skip_onboarding( WP_REST_Request $request ): WP_REST_Response {
$parameters = new Rsssl_Request_Parameters( $request );
// Un TryCatch pour récupérer les erreurs et retourner une 403
try {
$this->check_login_and_get_user($parameters->user_id, $parameters->login_nonce);
} catch (Exception $e) {
return new WP_REST_Response(['error' => $e->getMessage()], 403);
}
return $this->authenticate_and_redirect( $parameters->user_id, $parameters->redirect_to );
}
private function check_login_and_get_user( int $user_id, string $login_nonce ) {
if ( ! Rsssl_Two_Fa_Authentication::verify_login_nonce( $user_id, $login_nonce ) ) {
wp_die(); // CETTE FOIS CI, ON STOPPE L'EXECUTION si le nouce est invalide
}
$user = get_user_by('id', $user_id);
if (!$user) {
throw new Exception('User not found'); // Cerise sur le gateau, on retourne une exception si l'utilisateur n'existe pas
}
return $user;
}
Les développeurs ont changé deux choses :
- Une meilleure stratégie de Fail Safe de la fonction check_login_and_get_user(). Si le nonce n’est pas valide, wp_die() coupe brutalement l’exécution. S’il y a un souci avec l’utilisateur, une exception est levée
- Puis le pendant, de ce Fail Safe, celui qu’on attendait : un try/catch autour de check_login_and_get_user(). En cas d’erreur, on renvoie une Forbidden
Je note quand même deux petits regrets :
- check_login_and_get_user() pourrait retourner un type strict ?WP_User
- Dans le même registre, à la place de wp_die(), on pourrait retourner une expcetion
- Au lieu d’utiliser $parameters->user_id, on pourrait passer directement $user->ID lors de l'appel de la fonction d'authentification
Je me pose une dernière question.
Au tout début de notre investigation, nous avons souligné que cette fonctionnalité de “Skip unboarding” était normalement cadrée temporairement. On ne devrait pas pouvoir “Skip” au bout de X jours après configuration du 2FA.
Sauf que nous n’avons pas vu une telle condition … Alors deux possibilités : Soit la condition n’existe pas (et pour moi, c’est un bug de sécurité) ou soit la condition est caché dans la partie vérification du nonce. Et alors je trouve que c’est caché de la complexité. Si du code est ambigu, alors le risque de bug augmente.
Voila, c’est tout pour cette première investigation AppSec !
Si ce genre d'investigation de sécurité applicative vous intéresse, n'hésitez pas à nous rejoindre sur les réseaux. Nous publierons d'autres investigations sur des vulnérabilités intéressantes !
