Outils pour utilisateurs

Outils du site


langages:php:erreurs

PHP

On ne met pas de virgule après le dernier élément d'un tableau

Faux ! Une virgule après le dernier élément d'un tableau est parfaitement valide syntaxiquement :

$foo = array(34 => 'xxx',);
$bar = [ 'xxx' => 78, ];

C'est couramment utilisé quand les tableaux sont créés sur plusieurs lignes : pour insérer une nouvelle clé/valeur sur une ligne, nous n'avons alors pas à reprendre la ligne précédente pour lui rajouter la virgule omise au départ.

La documentation, par ailleurs, le mentionne très bien :

La virgule après le dernier élément d'un tableau est optionnelle et peut ne pas être ajoutée. C'est généralement ce qui est fait pour les tableaux sur une seule ligne, i.e. array(1, 2) est préféré à array(1, 2, ). Pour les tableaux sur plusieurs lignes, la virgule finale est généralement utilisée, car elle permet d'ajouter plus facilement de nouveaux éléments à la fin.

Insertion des données sous forme htmentities/htmlspecialchars

Il est vrai qu'il est tentant de stocker ses données après leur avoir appliquées htmlentities ou htmlspecialchars de sorte de ne pas avoir à se soucier des failles XSS par la suite. En réalité, cette démarche pourrait, d'une part, être fausse dès que vous réaffichez d'autres données qui ne sont pas d'abord passées par votre base de données ; ce qui serait exceptionnel et risquerait de vous faire prendre de mauvaises habitudes (= failles). D'autre part, se pose la question de la dénaturation des données. Que ferez-vous si vous devez effectuer des recherches dans votre base de données ou encore générer, à partir de celle-ci, des formats non HTML (PDF, Excel, XML, graphique/png, etc) ? Un html_entity_decode à chaque fois ? De même, si vous exploitez cette base à partir d'autres clients (Java, C#, etc), cela vous oblige à rester cohérent donc à trouver et appliquer un équivalent de htmlentities/htmlspecialchars à chacune de leur insertion/modification et html_entity_decode à chaque lecture ? Que de travail inutile !

Notez également que é par rapport à é consomme inutilement 8 fois plus de points de code (types (VAR)CHAR) et 8 à 4 fois plus d'octets suivant le jeu de caractères (types *TEXT) pour son stockage.

En tant que bon français, une petite illustration du travail supplémentaire et totalement inutile alors demandé par cette htmlentitiesation en prenant simplement un caractère comme œ et une collation MySQL utf8_unicode_ci où œ = oe (plus insensibilité à la casse).

À cette occasion, créons cette table :

CREATE TABLE test(
    id INT UNSIGNED NOT NULL AUTO_INCREMENT,
    string VARCHAR(80) CHARACTER SET utf8 COLLATE utf8_unicode_ci NOT NULL,
    PRIMARY KEY(id)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;

Afin de travailler dans un premier temps sur des données htmlentitisées, insérons ces quelques lignes représentant les différents cas à gérer :

INSERT INTO test(string) VALUES('noeud');
INSERT INTO test(string) VALUES('nœud');

La requête, pour obtenir ces résultats doit alors demander deux critères au lieu d'un seul :

SELECT * FROM test WHERE string LIKE '%œ%' OR string LIKE '%oe%'
-- ou
SELECT * FROM test WHERE string REGEXP '(œ|oe)'

Ce qui revient à court-circuiter la collation. Et encore, c'est approximatif car "oe" pourrait être trouvé dans une entité HTML. Citons notamment l'entité MathML ⤨. Ce n'est qu'une question de logique : comment voulez-vous indexer (recherches fulltext par exemple) des données htmlentitiesées ?

Remplacez maintenant le contenu de cette table par ces lignes :

INSERT INTO test(string) VALUES('noeud');
INSERT INTO test(string) VALUES('nœud');

Vous remarquez alors que si nous avions gardé notre texte intact, ce serait inné et la recherche est assurément cohérente :

SELECT * FROM test WHERE string LIKE '%œ%'

Conclusion, ne pas appliquer htmlentities à l'insertion pour :

  • assurer la cohérence des recherches (plus le texte à chercher sera court plus ce sera flagrant)
  • s'épargner du travail inutile
  • le seul recours viable aux failles XSS reste l'htmlentitiesation systématique à l'affichage

Note : des caractères comme œ ou € n'existent pas en ISO-8859-1 (ni en CP850). Assurez-vous de choisir un jeu de caractères (base de données comme php) qui couvre vos besoins (mais ne perdez pas de vue ce qu'un tel changement implique parfois). Dans le cas d'UTF-8, on notera que htmlspecialchars suffit [est à préférer] à htmlentities.

Les deux-points des marqueurs PDO pour les méthodes execute, bindValue et bindParam sont obligatoires

Contrairement à ce que beaucoup prétendent à défaut de l'avoir vérifié ou testé (et ça se dit utilisateur de PDO) : les deux points (:) pour les noms des marqueurs dans les appels des méthodes execute, bindValue et bindParam sont et ont toujours été facultatifs : leur éventuelle absence est gérée depuis 2005 par la révision 1.116, bien avant l'intégration de PDO au core (version 5.1.0).

Que les personnes de mauvaise foi ne viennent pas surenchérir qu'il est conseillé de les ajouter pour de meilleures performances : c'est faux dans la mesure où, qu'ils soient ou non mis, la chaîne fournie, correspondant au nom du marqueur, est dupliquée (nouvelle allocation).

PDO est plus sécurisé

Faux ! PDO n'offre en rien plus de sécurité que quoi que ce soit d'autre. Il en offre ni plus ni moins. Cette fausse idée est véhiculée par des ignorants attribuant à tort l'exclusivité des requêtes préparées à PDO. Or il n'en est rien : les requêtes préparées sont normalement implémentées au niveau même du client du SGBD (libmysql ou mysqlnd pour MySQL par exemple) sous-jacent à l'extension PHP. En outre, on les retrouve dans d'autres extensions/clients : mysqli, sqlite3, etc.

Par ailleurs, n'oublions pas que, même si PDO, et toute extension interfaçant les requêtes préparées, permet toujours également (et heureusement) des requêtes non préparées.

Conclusion : la sécurité reste purement à la charge du développeur : PHP ou le SGBD ne pourront jamais s'en charger pour vous. Et ce, que la requête soit préparée ou non. En effet, une requête préparée (vraiment) mal utilisée pourrait toujours permettre une injection, chose que l'on voit de temps en temps de la part des débutants. Le tout est de bien différencier le type de la requête (préparée ou non) et dans le second cas (non préparé) d'échapper ce qui doit l'être avec la fonction prévue à cet effet (par le client du SGBD) :

  • mysql ⇒ mysql_real_escape_string
  • mysqli ⇒ mysqli(::|_)real_escape_string
  • pdo ⇒ PDO::quote
  • etc

Je dois serialize mon objet pour le mettre en session

TODO

Je dois stripslashes les données lues de la base (SELECT)

Désolé mais on ne devrait justement aucunement avoir à retirer aucun échappement à la (re)lecture de ses données issues d'une base de données. stripslashes ne devrait pas être nécessaire à ce moment.

Certes, on doit échapper, en non-préparé uniquement, pour la sécurité (injections) et assurer la validité de la syntaxe de la requête, les données en entrée (lors d'une insertion ou mise à jour notamment) mais cet échappement n'est que temporaire : il est destiné à la communication client/serveur, disons, mais il n'apparaît plus au niveau des données stockées. Trouvez-vous logique, quand vous insérez "l'arbre", de récupérer "l\'arbre" ? Non !

Si vous avez besoin d'utiliser stripslashes, c'est certainement parce que vos données sont échappées deux fois :

  • une fois par vous-mêmes pour les injections (ce qui est déjà une bonne chose)
  • l'autre c'est PHP qui doit réaliser un échappement automatique sur toutes les variables externes du script ($_GET, $_POST, $_COOKIE). En effet, ça se configure au niveau de la directive magic_quotes_gpc, qui active (valeur on), effectue automatiquement un échappement générique.

Il faut cependant savoir que magic_quotes_gpc ne peut se désactiver au niveau même du script (ini_set) car PHP réalise (si magic_quotes_gpc est à on) l'échappement avant l'exécution du script. Pour le désactiver, il faut un accès au php.ini sinon ça dépend essentiellement de son mode d'exécution (CGI vs module Apache) puis de ce que votre prestataire (pour ceux qui doivent se contenter d'être mutualisé) vous permet ou non. Vraiment au pire, on peut appliquer automatiquement un stripslashes en début de ses scripts, par exemple :

if (function_exists('get_magic_quotes_gpc') && get_magic_quotes_gpc()) {
    $gpc = array(&$_GET, &$_POST, &$_COOKIE, &$_REQUEST);
    array_walk_recursive($gpc,
        // PHP >= 5.3
        function (&$value, $key) {
            $value = stripslashes($value);
        }
        // PHP < 5.3
        //create_function('&$value, $key', '$value = stripslashes($value);')
    );
}

Cette immondice, dépréciée depuis la version 5.3.0 et qui disparaît définitivement avec la version 5.4.0 devrait être désactivée, car, comme largement plus expliqué dans la partie intitulée "J'utilise addslashes ou magic_quotes_gpc pour me prémunir des injections SQL", ce n'est pas à PHP de réaliser un échappement générique, donc qui ne convient pas.

Pour ceux qui auraient besoin d'une "démonstration" :

<?php
try {
    $dbh = new PDO(DSN, LOGIN, MOT_DE_PASSE);
 
    $dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
    $dbh->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_OBJ);
 
    if ($_SERVER['REQUEST_METHOD'] == 'POST') {
        if (get_magic_quotes_gpc()) {
            $texte = stripslashes($_POST['texte']);
        } else {
            $texte = $_POST['texte'];
        }
        $query = 'INSERT INTO test_utf8(string) VALUES(' . $dbh->quote($texte) . ')';
        var_dump($_POST['texte'], $texte, $query, $dbh->exec($query));
 
        $stmt = $dbh->query('SELECT * FROM test_utf8 WHERE id = LAST_INSERT_ID()');
        var_dump($stmt->fetch());
    }
 
} catch (Exception $e) {
    die(sprintf("%s dans %s à la ligne %d : %s", get_class($e), $e->getFile(), $e->getLine(), $e->getMessage()));
}
?>
 
<form method="post" action="">
    <input type="text" name="texte" value="" />
    <input type="submit" value="Envoyer" />
</form>

En insérant la chaîne "l'escargot 1\ 2
3\
\r\n", on obtient pour sortie :

string(36) "l\'escargot 1\\ 2\\\\ 3\\\\\\ \\r\\n"
string(27) "l'escargot 1\ 2\\ 3\\\ \r\n"
string(76) "INSERT INTO test_utf8(string) VALUES('l\'escargot 1\\ 2\\\\ 3\\\\\\ \\r\\n')"
int(1)
object(stdClass)#7 (2) {
  ["id"]=>
  string(2) "20"
  ["string"]=>
  string(27) "l'escargot 1\ 2\\ 3\\\ \r\n"
}
  1. La première chaîne il y a sur-échappement (par PHP), puisque j'ai activé les magic_quotes_gpc exprès pour vous
  2. La deuxième c'est ma chaîne telle qu'elle est écrite (stripslashée s'il y avait les magic_quotes_gpc)
  3. La troisième, c'est la requête, telle qu'elle sera exécutée. On retrouve les échappements par/pour MySQL. (et c'est ce que je retrouve avec le profiler de mysql)
  4. Le int(1) prouve que la requête s'est exécutée normalement
  5. L'objet stdClass c'est parce que je suis en mode PDO::FETCH_OBJ, mais on voit bien que les données à la récupération sont strictement identiques à celles de départ

Note : j'utilise PDO pour la démonstration mais vous aurez strictement le même résultat avec mysql comme avec mysqli.

J'utilise addslashes ou magic_quotes_gpc pour me prémunir des injections SQL

Il faut bien prendre conscience que ce n'est pas à PHP de réaliser un échappement générique. Pourquoi ?

  • Tout d'abord il ne tient aucunement compte du jeu de caractères des données à échapper. Chose qui est susceptible d'engendrer une faille.
  • Enfin, l'usage fait de ces données peut demander une façon d'échapper qui est différente : on n'échappera pas de la même manière une donnée utilisée/injectée dans un motif d'expression régulière PCRE (preg_quote) qu'un SGBD (et encore, dans ce dernier cas, ça dépend justement du SGBD).

Pour être plus concret et critique, rappelons l'avis de sécurité relatif à l'encodage pour MySQL :

Security Fix: An SQL-injection security hole has been found in multi-byte encoding processing. The bug was in the server, incorrectly parsing the string escaped with the mysql_real_escape_string() C API function.

This vulnerability was discovered and reported by Josh Berkus <josh@postgresql.org> and Tom Lane <tgl@sss.pgh.pa.us> as part of the inter-project security collaboration of the OSDB consortium. For more information about SQL injection, please see the following text.

Discussion. An SQL injection security hole has been found in multi-byte encoding processing. An SQL injection security hole can include a situation whereby when a user supplied data to be inserted into a database, the user might inject SQL statements into the data that the server will execute. With regards to this vulnerability, when character set-unaware escaping is used (for example, addslashes() in PHP), it is possible to bypass the escaping in some multi-byte character sets (for example, SJIS, BIG5 and GBK). As a result, a function such as addslashes() is not able to prevent SQL-injection attacks. It is impossible to fix this on the server side. The best solution is for applications to use character set-aware escaping offered by a function such mysql_real_escape_string().

Notez également, pour conserver MySQL en exemple, que ce dernier possède deux modes d'échappement :

  1. celui que tout le monde connaît, je pense, qui consiste à précéder tout caractère problématique (\, ', " notamment) d'un antislash
  2. l'autre, nommé NO_BACKSLASH_ESCAPES, qui considère les antislashs comme des caractères normaux et où les quotes sont alors doublées

Le mode d'échappement de MySQL se configure à la volée (à la demande du client) ou côté serveur (global ou non).

Il n'y a que le client du SGBD (PDO::quote, mysql_real_escape_string, mysqli(::|_)escape_string) qui puisse dès lors réaliser un échappement correct : il sait, contrairement à PHP (seul : addslashes/magic_quotes_gpc), quel est l'encodage courant ainsi que le mode d'échappement à considérer pour échapper réellement les données destinées au serveur.

Vous pouvez effectuer le test suivant pour voir la différence de ces deux modes :

<?php
$dbh = new PDO('mysql:host=' . DBHOST . ';dbname=' . DBNAME, DBUSER, DBPASSWD);
 
$dbh->exec('SET PROFILING = 1');
 
$pattern = '~\[color=([\'"]?)([a-z]+|#[[:xdigit:]]{6})\1\](.+)\[/color\]~Usi';
$replacement = '<span style="color: \2">\3</span>';
 
$dbh->exec('INSERT IGNORE INTO motifs(pattern, replacement) VALUES(' . $dbh->quote($pattern) . ', ' . $dbh->quote($replacement) . ')');
$insert1 = $dbh->prepare('INSERT IGNORE INTO motifs(pattern, replacement) VALUES(:pattern, :replacement)');
$insert1->execute(array('pattern' => $pattern, 'replacement' => $replacement));
 
$dbh->exec('SET PROFILING = 0');
$dbh->exec('SET sql_mode = "NO_BACKSLASH_ESCAPES"');
$dbh->exec('SET PROFILING = 1');
 
$dbh->exec('INSERT IGNORE INTO motifs(pattern, replacement) VALUES(' . $dbh->quote($pattern) . ', ' . $dbh->quote($replacement) . ')');
$insert1->execute(array('pattern' => $pattern, 'replacement' => $replacement));
 
$dbh->exec('SET PROFILING = 0');
 
$prof = $dbh->query('SHOW PROFILES');
print_r($prof->fetchAll(PDO::FETCH_ASSOC));

header et setcookie fonctionnent même après un envoi de contenu

Si cela est possible c'est que votre PHP est configuré pour mettre en tampon sa sortie (output_buffering actif). Cette sortie n'est alors pas envoyée immédiatement (uniquement en fin de script ou quand ce tampon est rempli) et vous a alors permis d'envoyer vos entêtes. De fait, ça n'en fait pas un comportement portable : sur tout autre serveur, il ne sera pas garanti que ce même script fonctionne (output_buffering à off). Au passage, étant donné que ce n'est pas le comportement "normal", on pourrait considérer qu'il y a là une erreur de conception.

Les expressions régulières (PCRE) sont plus rapides que les fonctions str* (standard)

En pratique c'est faux. On trouve des rigolos qui n'y connaissent rien au fonctionnement de PHP et qui sont tout content de faire un benchmark avec 10000 itérations (par exemple) pour conclure que PCRE est plus rapide. Dans ce cas de figure, c'est vrai mais qui fait 10000 fois une même recherche couramment dans un même script ? Le fait est que PHP, et plus précisément l'extension PCRE, met en cache la "compilation" de l'expression régulière (l'automate généré étant plus performant). Donc forcément, à partir d'un nombre assez faible d'itérations l'expression régulière PCRE, en terme de performances, va prendre le dessus sur les fonctions standard de manipulation des chaînes de caractères de PHP qui ne sont pas vraiment optimisées car elles ne sont pas prévues pour des utilisations répétées (sinon, il y a des algorithmes type KMP pour ça avec un pré-traitement). Et plus ce seuil sera dépassé, moment où la compilation est amortie, plus l'écart entre les fonctions de PHP et PCRE sera significatif.

Parce qu'à partir du moment où votre motif est strictement le même (comprend les modificateurs) ainsi que la locale, que PHP a déjà compilé l'expression, vous allez récupérer ce résultat, elle ne sera plus recompilée. Et ce cache est commun à toutes les fonctions PCRE (preg_match, preg_replace, etc).

⇒ Faire la comparaison est dès lors complètement stupide, surtout sans prendre en compte le contexte !

Les noms des fonctions/méthodes sont sensibles à la casse

Il FAUT un try/catch

Avant de faire allusion à un try/catch, la première chose à faire est de considérer le contexte : il ne faut pas confondre erreurs et exceptions. Un bloc try/catch interceptera bien la levée (throw) d'une exception mais certainement pas une erreur. Par exemple, ce code ne fera strictement rien et va même générer de jolis E_WARNING si log.txt n'existe pas :

try {
    $fp = fopen('log.txt', 'r');
    while (FALSE !== $line = fgets($fp))) {
        // ...
    }
    fclose($fp);
} catch (Exception $e) {
    echo "L'ouverture du fichier a échoué : ", $e->getMessage();
}

Du moins, pas par défaut, c'est-à-dire sans redéfinir un gestionnaire d'erreur qui transformerait l'erreur en une exception :

set_error_handler(
    function ($errno, $errstr, $errfile, $errline) {
        throw new \ErrorException($errstr, 0, $errno, $errfile, $errline);
    }
);

La distinction erreur/exception étant rappelée, je souhaitais aborder la fameuse nécessité des blocs try/catch souvent évoquée à tort et à travers. Encore un mythe complètement absurde ! Tout d'abord, try/catch n'a rien d'obligatoire : si une exception n'est pas gérée, que ce soit par un bloc try/catch comme un gestionnaire d'exception (cf fonction set_exception_handler), PHP la transforme tout simplement en une erreur fatale. D'ailleurs, ne pas gérer une exception est bien plus censé que de mettre un try/catch où le catch fait simplement un die du message de l'exception, ce qui est carrément contre-productif par rapport au comportement par défaut de PHP qui est, quand il rencontre une erreur (notre exception non gérée), de la consigner dans le journal d'erreur et éventuellement (suivant la valeur de display_errors) l'afficher.

try {
    # ...
} catch (Exception $e) {
    die($e->getMessage());
}

Ci-dessus, typiquement le try/catch complètement inutile, pour les adeptes du "je mets un try/catch pour en mettre un".

Ma base de données est en UTF-8 mais je fais un utf8_decode à l'insertion

Plus débile, absurde et incohérent, tu meurs ! Point positif, là-dedans, celui qui fait ça aura au moins compris que le jeu de caractères par défaut côté client du SGBD (pour rappel c'est PHP le client ici) est de l'ISO-8859-1 ou une variante (encore que ce serait inexact : ISO-8859-1 n'existe pas par exemple pour MySQL, latin1 correspond au jeu CP1252, qui aurait permis la représentation de caractères supplémentaires comme € ou œ).

Première erreur, la moindre : c'est qu'il hérite du jeu de caractères par défaut et s'en contente au lieu d'expliciter une bonne fois pour toute celui sur lequel repose toute son application (c'est juste une ligne de code à ajouter ou modifier). En quoi est-ce déconseillé ? La raison est simple : ce jeu par défaut, il se configure (dépend du SGBD qui dispose éventuellement de plusieurs moyens), le jour où vous faites une mise à jour du SGBD ou que vous travaillez sur une machine différente, qu'est-ce qui vous dit dès lors que ce jeu par défaut ne sera pas différent ? Vous faites là un gros pari où vous risquez de payer très cher la ligne de code que vous n'avez pas voulu ajouter/modifier.

Deuxième erreur, la plus grave : l'auteur de cette abomination, ne peut qu'ignorer ce qu'est un jeu de caractères et leurs buts (ce qu'ils permettent de représenter - ou non). Un jeu de caractères comme ISO-8859-1, c'est 256 caractères quand Unicode, dont le but est de pouvoir tout représenter, c'est un peu plus de 100 000 points de code assignés. La différence est-elle plus claire à présent ? Donc par utf8_decode, vous basculez du jeu de caractères UTF-8 à ISO-8859-1 mais comme ce dernier est beaucoup beaucoup … beaucoup plus limité, vous perdez tout bonnement tout caractère présent en Unicode mais pas en ISO-8859-1 (chinois, cyrillique, etc) (soit environ 99,75 % de perte théorique). Encore une fois, le problème est simple à résoudre : virer ce foutu utf8_decode et ajouter la fameuse instruction SET NAMES utf8 (ou, mieux, équivalent client quand disponible) qu'il vous manque. Est-ce si difficile d'ajouter/modifier une ligne de code ? Sinon, quel intérêt à utiliser Unicode/UTF-8 pour tout permettre au niveau du site si juste avant l'insertion, c'est pour supprimer tout caractère non latin (en gros) alors que la base de données est parfaitement capable et initialement prévue pour stocker de l'UTF-8 ? C'est purement aberrent. N'utilisez pas Unicode/UTF-8 si vous ne savez pas en faire bon usage, comme dans ce cas de figure !

Suis-je concerné ? Très simple à déterminer :

  • chercher utf8_decode dans vos scripts (facile à réaliser sur Unixoïde via grep par exemple)
  • tenter d'insérer, par l'intermédiaire de votre site, un caractère non latin (chinois, japonais, peu importe), s'il est remplacé en base ou au SELECT par un point d'interrogation, vous l'êtes

Apache

Réécriture : le flag L(ast) met fin à la réécriture

Incompréhension courante au sujet du flag L(ast) est de croire qu'il met fin à toute réécriture or ce n'est pas le cas. Ceci, par contre, est la description du flag END, introduit par Apache 2.4.0. Le flag L(ast) ne met fin qu'au processus courant de réécriture, il faut voir ça comme une fin temporaire.

C'est-à-dire que si Apache matche une règle flagguée L(ast), il va l'appliquer tout de suite au lieu de continuer à chercher des correspondances avec les règles restantes mais, la destination de cette règle appliquée, va à son tour subir toutes les règles de réécriture qui lui sont applicables (et ainsi de suite).

Forcément, dans ces circonstances, l'erreur que les gens ne comprennent pas, c'est que pour une réécriture située dans un .htaccess, si l'application d'une première règle reconduit au même répertoire, signifie que l'on en retrouve ses règles, d'où potentielle boucle infinie (erreur 500 avec "Request exceeded the limit of 10 internal redirects").

C'est par exemple le cas de RewriteRule (.*) index.php?page=$1 (placée dans un .htaccess ou bloc <Directory>), qui, avec ou sans flag L(ast), va (éventuellement) produire la boucle suivante : /foo ⇒ index.php?page=foo ⇒ index.php?page=index.php ⇒ index.php?page=index.php ⇒ index.php?page=index.php et ainsi de suite.

MySQL

LIKE est insensible à la casse

langages/php/erreurs.txt · Dernière modification: 08/05/2015 17:50 de julp