Défi sécurité.

[Amrac]

Salut,
Histoire de placer le contexte, je suis webmaster du jeu Fourmizzz.fr, j'ai une communauté de 11 988 joueurs dont 3 260 actifs et je dois dire qu'ils m'en font parfois voir des vertes et des pas mures au niveau sécurité.

J'aimerais vous faire partager mon expérience (douloureuse) de ce week-end à travers un petit défi.

Je vais donc vous présenter un petit bout de code qui permet d'annuler une attaque si celle-ci a était lancé il y a moins de deux minutes.
Dans ce petit code a priori inoffensif se cache une faille majeur, je vous met donc au défi de la trouver.

J'ai un peu sur-commenté chaque ligne histoire de vous simplifiez la compréhension du code.

Code PHP :

<?php

//Dans la variable $ID se trouve l'ID du joueur



/* ******  Annulation d'une attaque: ****** */

if(isset($_GET['annuler'])) //On vérifie s'il y a eu une demande d'annulation

{

$result = mysql_query("SELECT * from attaque WHERE IDAttaquant='$ID' AND ID='".$_GET['annuler']."'");//Si oui, on sélectionne l'attaque que l'ont doit annuler dans la base de données.

$donnee = mysql_fetch_array($result);

if($donnee['timestampDepart']>(time()-2*60))//Timestamp départ contient le timestamp au moment du lancement, on vérifie donc que cela fait moins de deux minutes que le lancement de l'attaque a eu lieu.

 {

    $Armee=getSoldats($donnee);//On extrait des donnees les informations décrivant l'armée. Pour vous simplifier la vie, on considére cette fonction sans faille.

    addArmee($ID,$Armee,'2');//On remet l'armée en question au joueurs qui vient d'annuler l'attaque. Pour vous simplifier la vie, on considére cette fonction sans faille.

    mysql_query("DELETE from attaque WHERE IDAttaquant='$ID' AND ID='".$_GET['annuler']."'");//On supprime l'attaque de la base de données.

 }

}



/* ******  Affichage des attaques: ****** */

$resultat = mysql_query("SELECT * from attaque WHERE IDAttaquant='$ID'");



while ($donnees = mysql_fetch_array($resultat))

{

    $tempsRestant = $donnees['timestamp'] - time();



    echo '<p class="gras">- Vous allez attaquer '.getPseudo($donnees['IDDefenseur']).' dans '.convertiTemps($tempsrestant);//Message de l'attaque.

    if($donnees['timestampDepart']>(time()-2*60)) //Si le joueur peut encore annuler l'attaque, alors on ajoute le lien pour pouvoir annuler.

    {

        echo ' - <a href="Armee.php?annuler='.$donnees['ID'].'">Annuler</a>';

    }

    echo ' </p>';



} 

[NicoMSEvent]

je pense avoir une piste ^^

[Plume]

Une accolade en trop à la fin ? iffle:

[orditeck]

Je t'envoi un MP

[Plume]

En même temps, si j'ai trouvé, ça va. C'était assez simple

Mais c'est sympa comme idée

[X-ZoD]

le get ? -_-

[Amrac]

Je suis étonné de la participation, je pensais que le sujet allé coulé vite fait dans les oubliettes

Une accolade en trop à la fin ? iffle:

J'ai oublié de l'enlever, merci ^^

Je vous fait passer les MPs qui n'ont pas aboutie, et le complément d'information qui va avec:

tu as eu une injection de SQL dans la variable : $_GET['annuler'] ?

Pouratant, une des premiere chose qu'on apprend en sécurité, est de ne jamais faire confiance aux données entrées... faut tester la validité, faire des "escape" et tout ce qu'il faut pour éviter ça...

Si ce n'est pas ça, je te suggère quand meme de vérifier ce point 16

Règle numéro 1 en PHP : ne jamais utiliser GET.

Est-ce là toute l'erreur de ton script ? ou bien il y a quelque chose d'encore plus profond à trouver ?

Kéké.

pas certain, mais étant donné la configuration des fonctions et leur point d'entrées, je soupçonne de l'injection SQL dans la variable $_GET['annuler']

par exemple avec un "0 OR IDAttaquant = (Id d'un autre joueur)"

on pourrait alors provoquer l'annulation d'une attaque récente de la part d'un autre joueur dont on aurait récupéré l'ID.
Bien entendu, simplement vérifier que $_GET['annuler'] est un nombre rend caduque cette attaque.

par contre comme je ne vois pas trop le lien avec la seconde partie du script je ne suis pas sûr de mon coup..

Déjà, comme dis Lex, y'a une accolade de trop à la fin ce qui résulte à une erreur PHP.
Je vois pas comment tes joueurs peuvent afficher cette page 1

Sinon, tu fais un :
DELETE from attaque WHERE IDAttaquant='$ID' AND ID='".$_GET['annuler']."'

Ensuite, un joueur fait un petit :
Armee.php?annuler=1' DROP DATABASE 'jeu

Et le tour est joué... enfin, ça fonctionne seulement si magic_quotes_gpc est désactivé. Mais pour des raisons pratiques, je crois que c'est mieux qu'il soit activé.

S'il n'est pas activé, voici un petit bout de code utile qui arrange tout ça, bien que j'imagine que ta faille est corrigée depuis longtemps... !

Code PHP :
$get_arr = array();
foreach ($_GET as $key => $get_arr) {
$_GET[$key] = addslashes($get_arr);
}


J'ai trouvé, au moins ?

Pour l'injection SQL, vous avez raison de penser à cela, c'est de ma faute car je ne vous est pas fournit suffisament d'informations.
J'ai le magic_quote d'activé, donc l'injection n'est ici pas possible.
De plus, j'utilise une couche au dessus de mysql_query que j'ai retiré pour vous simplifier les choses.
Pour chaque requete, si il y a une erreur SQL elle n'est pas affiché à l'utilisateur (il ne connait donc rien de la structure de la base de données). De plus, un message privé m'est envoyé avec l'IP, l'ID, l'heure, toutes les variables en GET/POST, et le nom du script. De la sorte, ceux qui voudraient faire des tentatives n'ont aucun droit à l'erreur.
Mais vous avez eu raison de soupconner ce point avec les informations que vous aviez

autre point que je vois, c'est : Armee.php?annuler='.$donnees['ID']

Rien n'empeche de sauver le lien, et le lancer plus tard. A moins que tu ne vérifies si la durée de validité du lien est ok lorsque qu'il est activé (avec un message du genre : 'trop tard!' si il n'est plus valide).

Si l'attaque à déjà était annulé, et que tu ré exécute le script plus tard, la requete SELECT * from attaque WHERE IDAttaquant='$ID' AND ID='".$_GET['annuler']."' ne selectionnera rien puisque l'attaque n'existeras plus, le script va ajouter 0 armée.
Il y a de l'idée aussi.

Bonjour 2

Le problème ne serait pas que le fait de passer l'id de l'attaque à annuler en GET permet au joueur d'annuler une attaque qui n'est pas la sienne ?

L.

Dans la requete qui selectionne l'attaque, il y a deux conditions: ID='".$_GET['annuler']."' pour selectionner l'attaque du GET, et IDAttaquant='$ID', $ID correspondant a l'ID du joueur qui à lancé l'attaque et qui exécute le script.
Si un joueur met dans $_GET['annuler'] l'ID d'une attaque qui ne lui appartient pas, la seconde condition ne sera pas vérifier, il n'y aurat donc aucune attaque de selectionné.
Je précise accesoirement que le champ ID de attaque est en auto-incremente (il est donc unique).


Edit: Je me suis un peu emmêlé les pinceaux avec les quotes

[keke]

Donc si je comprends bien ton message, il faut qu'on creuse encore un peu ?

Je continue.

kéké.

[Amrac]

Donc si je comprends bien ton message, il faut qu'on creuse encore un peu ?

Je continue.

kéké.

Oui ^^

si tu de délogue, le $ID n'est plus initialisé... il ne suffirait pas de rajouter a n'importe quelle requete un parametre en GET &ID=xx pour usurper l'identité de n'importe quel joueur?

Le script n'est accessible que si on est connecté. (Il y a un système de session qui s'en occupe)
Il ne s'agit pas d'un problème de session.

Je vous laisse un peu chercher, et dans quelques heures je vous donnerais quelques indices pour vous mettre sur la piste.

[pascal]

petite question de configuration :
register_globals vaut On ou Off ?

A+

Pascal