Closed J9rem closed 6 months ago
Salut ! le code me va bien, par contre je comprend pas contre quel attaque ça défend, tu peux me donner un example dans le cas de la mise à jour?
Alors en fait, c'est une bonne pratique de toujours filtrer les $_GET
et $_POST
.
Imaginons que nous ayons oublié d'attraper à un autre endroit de YesWiki une injection de code PHP
(c'est de moins en moins probable avec l'usage de twig
mais il peut rester des coins cachés).
L'injection de code php peut en théorie modifier les fichiers php
(vu que nous avons besoin des droits d'écriture pour faire les mises à jour en ligne) et peut ajouter du contenu dans $_GET
comme un objet au lieu d'une chaîne de caractère.
Cet objet peut alors exécuter du code malsain en mode connecté administrateur avant de rendre un truc qui ressemble à une chaîne de caractère.
Le test $_GET['action'] == 'update'
ne permet que de vérifier que le truc stocké dans $_GET['action']
vaut 'update'
.
Mais si un script malveillant qui aurait réussi à copier du code dans YesWiki.php
modifie $_GET['action']
pour que ce soit un objet, alors le test $_GET['action'] == 'update
va se traduire par $_GET['action']->__toString() == 'update'
qui peut être vrai, mais l'objet malveillant peut faire beaucoup de chose avant de terminer __toString()
.
Par précaution, il vaut mieux tester $_GET['action'] === 'update'
pour s'assurer que c'est une chaîne de caractère et non un objet et la deuxième précaution est de passer par filter_input
pour éviter que du code modifie $_GET['action']
et fasse croire que tout s'est bien passé alors qu'entre temps, il a fait plein de choses en mode administrateur.
Vois-tu la précaution que je propose vu que ce code s'exécute en mode admin ?
j'ai mergé, mais je ne vois toujours pas concrètement quel code tu pourrais mettre dans $_GET pour provoquer une injection avec $_GET['action'] == ''update"
Si on avait du code du type
<?php
$page = $_GET['page'];
include($page . '.php');
?>
ou
<?php
$id = $_GET['id'];
$sql = "SELECT * FROM users WHERE id = $id";
je comprends, mais sinon je vois pas...
Mais si un script malveillant qui aurait réussi à copier du code dans YesWiki.php
Si le script arrive à faire ça, alors il peut aussi bien écrire une post action __UpdateAction.php
et faire tout le code qu'il veut
@seballot comme convenu, je te propose cette PR. Elle est en plusieurs commits
SecurityController::filterInput
FILTER_SANITIZE_STRING
est dépréciée à partir dephp 8.1
)PSR2
$_GET
dansautoupdate
par un appel defilterInput
$_GET
et qu'aucun script ou extension ne pirate le contenu de$_GET
dans l'idée de déjouer les sécuritésYesWikiMigration::getService
pour faciliter les chosesQu'en penses-tu ?