N’aidez pas les pirates et arrêtez de tout inclure !

J'en avait déjà parlé de vive voix avec quelques personnes lors d'un précédent WordCamp il me semble. Je parle là d'un code qui semble smart, permettant d'inclure automatiquement les fichiers .php contenus dans un dossier d'un plugin ou d'un thème.

Smart code

Le but de ce genre de code est de pouvoir inclure tous les fichiers du dossier /inc/ par exemple, et cela sans devoir ajouter de ligne du style include( MY_PATH . '/class-new-fiture.php' );.

Comme vous le savez, un bon développeur est un développeur fainéant, moins le code est redondant, moins le code est à retoucher et maintenir, plus le code est clean et smart.

Bien, sauf qu'ici, l'idée est mauvaise, non mais vraiment, elle est aussi bonne que mauvaise.

Voici le code, il en existe d'autres dans pleins de thèmes et plugins, je prends celui-ci en exemple car c'est à cause de lui qu'un hack a été permis sur le site d'un de mes clients :


/**
* load all php files in one folder, if the folder contains files with different file extensions return the filenames as array
*
* @param string $folder path to the folder that should be loaded
* @return array $files files the folder contains that are no php files
*/
if(!function_exists('avia_backend_load_scripts_by_folder'))
{
function avia_backend_load_scripts_by_folder( $folder )
{
$files = array();
// Open a known directory, and proceed to read its contents
if ( is_dir( $folder ) )
{
if ( $dh = opendir( $folder ) )
{
while ( ( $file = readdir( $dh ) ) !== false)
{
if('.' != $file && '..' != $file)
{
$pathinfo = pathinfo($folder ."/". $file);</p>
<p>if( isset($pathinfo['extension']) && $pathinfo['extension']  == 'php' )
{
include_once( $folder ."/". $file );
}
else
{
$files[] = $file;
}
}
}
closedir($dh);
}
}</p>
<p>return $files;
}
}
Code from Velvet Theme

Le code est propre, commenté, indenté, avec un function_exists(), des if et des isset(), parfait !

Il suffit alors à son auteur de déposer d'autres fichiers dans les dossiers prévus à cet effet, et bingo tout sera inclus, rien d'autre à faire !

Le thème utilise la fonction comme ceci :


// ...
avia_backend_load_scripts_by_folder( AVIA_PHP . 'css' );
// ...
avia_backend_load_scripts_by_folder( AVIA_PHP . 'js' );
// ...
Inclusion de scripts JS et styles CSS

Tout ce qui est .js et .css sera retourné pour inclusion correcte à la façon WordPress, et les fichiers .php seront eux inclus via le include_once(). Les fichiers .php peuvent justement être les filtres permettant l'ajout en queue de ces styles et scripts.

La bonne idée

L'idée est plutôt bonne non ? Gain de temps, pas de code à retoucher, génial on push en prod.

Sauf que là, l'auteur vient d'aider les pirates à hacker votre site. Comment ?

Imaginez une faille permettant d'ajouter ou uploader des fichiers .php dans ces dossiers. Mais que ces dossiers soient protégés via htaccess contre l'accès direct, un simple deny from all suffit.

Le pirate serait alors coincé, il a réussi à uploader un script php malicieux, mais impossible pour lui d'y accéder.

La faille ...

MAIS, puisque le pirate a réussi à l'uploader dans les dossiers /css/ ou /js/, la fonction auto-include du thème va automatiquement inclure son fichier malicieux. Merci cher auteur (ou pas).

Ou pas.

L'idée de base était bonne, mais ici il faut avoir le bon comportement à propos de la sécurité :

  • Est-ce que ce que je développe pourrait permettre une attaque ?
  • Aussi tordue qu'elle puisse paraître, mon script peut-il mettre en danger les utilisateurs ?

Vous vous devez d'y penser, vous vous devez de devenir votre propre hacker et vous pirater. Si vous ne pouvez ou ne savez pas, faites auditer vos scripts par des personnes capables de vous aider, au risque de distribuer au monde entier, dans votre produit, une faille de sécurité critique.

L'auteur du thème a été averti, si jamais vous découvrez ce genre de fonction qui auto-inclus les .php, n'hésitez pas à commenter ici, à alerter l'auteur en lui donnant ce lien, ça pourrait le convaincre et ainsi tout le monde en sort gagnant.