Comment empecher les failles de type GET

[tghpow]

Bonjour a tous.

Mon problème est simple:

Pour mon jeu j'affiche une liste de village, pour chaque village on peut cliquer dessus pour aller sur sa page de description.
Seulement pour ce faire, je passe en paramètre dans l'URL l'id du village pour le récupérer via GET par la suite.. Ça veut donc dire que n'importe qui peut s'amuser à se balader de villages en villages en changeant cette id.

Alors je pourrais sécuriser ce procédé en comparant l'id session du joueur par rapport au joueur ayant le village, mais je trouve pas ça assez bien.

Comment avez-vous fait vous?

[Sephi-Chan]

Le mieux, c'est de toujours faire des requêtes de type :

SELECT * FROM village WHERE id = ? AND player_id = ?;

Comme ça, tu as la sécurité sans rien faire : si l'utilisateur spécifie un village qui ne lui appartient pas, la requête va retourner un résultat vide.

[tghpow]

Tiens oui c'est pas bête du tout merci
C'est vrai que ça sécurise l'accès.

Mais en plus de ça, il y a t'il un autre moyen de faire le lien entre la liste et la page d’aperçu de l'item sans passer des paramètres dans l'url?

[Xenos]

Si le joueur ne peux voir que son village (autrement dit, il n'a qu'un village), alors pas besoin de paramètre GET:

SELECT * FROM village WHERE player_id = ? [LIMIT 1]

Si le joueur peut avoir plusieurs villages, alors tu peux utiliser un paramètre "GET" qui ne prend non pas le n° du village, mais le n° d'ordre du village dans la requète:

SELECT * FROM village WHERE player_id = ? [ORDER BY id ASC] LIMIT $_GET['numero'], 1

(Attention: l'exemple est une horreur niveau injection SQL, donc à ne pas utiliser tel quel, c'est pour le principe)
Tu peux aussi stocker ce n° (le $_GET[]) dans une variable de session, et permettre au joueurs d'incrémenter/décrémenter ce n°. Ainsi, aucune paramètre "GET" ne spécifie directement un numéro: les paramètres GET ne spécifient qu'un +1 ou -1 (et ce n'est même pas un +1/-1 de l'id du village, mais un +1/-1 du numéro du village dans la liste des villages possédés par le joueur).

Code PHP :

<?php 

$_SESSION['numero'] += (int)($_GET['increment']) - (int)($_GET['increment']); 

[niahoo]

Alors je pourrais sécuriser ce procédé en comparant l'id session du joueur par rapport au joueur ayant le village, mais je trouve pas ça assez bien.

C'est pourtant la solution la plus logique et l'implementation de Sephi est la plus simple. Si la requête ne renvoie rien, paf 404.

[tghpow]

Merci de vos réponses, je vais surement m'orienter vers la solution de séphi pour commencer.
Mais je garde la tienne sous le coude xenos.

[Akira777]

Je déterre un peu le sujet.


Mais j'ajouterai de bien toujours faire gaffe au typage des INT, si un INT doit toujours être positif, ne pas se gaufrer en mettant bien la colonne en UNSIGNED pour MySQL et au mieux utiliser un petit abs() sur le nombre (valeur d'argent, XP, ...)

[Xenos]

Je suis d'accord de mettre UNSIGNED dans le typage de colonne, mais le abs(), je suis contre: si la valeur stockée dans la BDD est positive, alors tout va bien et le abs() est inutile. Si la valeur stockée est négative, alors il y a eu un problème en amont: pourquoi prendre la valeur absolue d'une somme négative d'argent? Cela ne fait pas sens.

Le mieux est de stocker la donnée en UNSIGNED, et d'effectuer un test (si on le souhaite) pour s'assurer que la valeur retournée est positive. Si le test échoue (valeur négative), on lance une exception que l'on rattrape ailleurs pour corriger le tir.

Passer par la valeur absolue, c'est prendre l'énorme risque de rater les erreurs, et de croire à tort qu'on les a corrigées alors qu'on a juste pris la valeur absolue d'un truc qui n'a pas de sens.

[Akira777]

Je soutiens ton avis,

pour être plus précis et parler d'un cas concret :

J'ai un système de marché tout con, un joueur peut choisir un item à vendre et indiquer la quantité à vendre. Certains joueurs s'amusaient à indiquer un chiffre négatif sur la quantité d'objet, et j'ai choisi, mais cela ne tient qu'à moi, de transformer ce nombre négatif en absolu plutôt que de gérer l'erreur.

Etant donné que le retrait d'une vente sur le marché était "payant" en monnaie virtuelle, et bien c'était bien fait pour leur gueule. Voilà tout
Et on retrouve ce cas sur le transfert d'argent, l'utilisateur d'un objet en quantité multiple, etc.
Maintenant je contrôle ça en JS, mais bon... Je garde toujours cet état d'esprit de faire payer cher les petits malins.

Ca sort du sujet mais exemple tout con, le jeu utilise le bon vieux système d'include de page "index.php?page=inventaire", ceux qui tapent "index.php?page=admin" étaient bannis pour 24h. Et c'est très drôle de voir les p'tits gars m'envoyer un email en me disant qu'ils ont pas fait exprès d'aller dessus alors qu'un coup d'oeil sur les logs me disait qu'ils avaient essayés de se connecter à l'admin en tapant "admin / admin".

En voilà quoi... question de style.

[Xenos]

Ah ben ok, avec cette information, je vais vite foncer sur le forum, m'inscrire, et mettre comme avatar l'image



Bannissement à tous ceux qui afficheront cette image. Si je l'utilise comme avatar, peu de gens m'attaqueront dans le jeu...
C'est une belle faille CSRF.

Pour le abs, d'accord, mais je pense qu'il faudra quand même l'expliciter (commentaire?) dans le code, sinon, cela sera délicat à relire.

Code :

$quantite = abs((int) filter_input(INPUT_POST, 'quantite', FILTER_VALIDATE_INT));
// Negative quantity is not allowed here: abs will switch it to positive,
// so one testing/hacking the interface with negative values will be forced to sell the items