Discussion :

[LPA__]

Bonjour à tous

Depuis quelques temps, j'essaye de mettre au point un script php me permettant de bloquer toute attaque XSS en éditant toutes les pages php d'un répertoire, en encapsulant toutes variables ou fonctions dans une fonction appliquant htmlspecialchar. Pour trouver toutes les sorties possibles à sécuriser sans rien rater et sans prendre quoi que ce soit qui ne devrait pas, j'essaye de mettre au point une REGEX me retournant tout ce qui doit être sanitariser. (Après, je ne le fais pas en full auto, le script me remonte les chaines à traiter, et je valide manuellement ou pas le remplacement.)

Jusque là ça marche plutôt bien , elle sait dans quel cas on est dans un echo ou pas, elle sait faire la différence entre ce qui est entre ' de ce qui ne l'est pas, mais mon problème viens des doubles quotes ". Entre ", les $variables sont interprétées, il faut donc les sanitariser. Mais une fois que ma regex m'a renvoyé une variable, la regex recommence juste après. Sauf que par défaut, elle se considère comme n'étant pas entre double quotes, et donc, quand elle recroisera une double quote, elle interprétera les caractères suivant avec son filtre "tout ce qui est entre double quotes"

Dans l'exemple suivant :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
echo "Ceci est un texte avec des $variables qui sont $interpretes." . functionQuelconqueQuiRenvoieUneString($var) ;
autreFonction();
echo "Deuxième string.";

Le premier match me renvoie $variables, tout va bien.
Lorsque le deuxième match à lieu, il le fait à partir de ça :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
 qui sont $interpretes." . functionQuelconqueQuiRenvoieUneString($var) ;
autreFonction();
echo "Deuxième string.";

Et plus rien ne marchera, puisqu'en rencontrant la 'première' double quote, il va chercher la suivante. Ce qui retournerai $var, au lieu de functionQuelconqueQuiRenvoieUneString($var).
Je pourrai résoudre ce problème si en PCRE, on pouvait faire un lookbehind à longueur variable, mais ce n'est pas le cas.
Donc j'aimerai savoir si il existe de manière interne à PCRE, un moyen de "savoir" si l'on se trouve dans une string ou pas.

Le script tourne sous php7, mais les pages php à éditer sont ... dans un tas de versions différentes.



PS, pour ceux qui veulent la regex dans l'état actuel (en travaux) : https://regex101.com/r/fZ0fH4/1

[badaze]

Sacrée regexp que la tienne.

Une question. Pourquoi ne te limites tu pas à contrôler les paramètres passés aux différents scripts ? Plutôt que de te lancer dans ce travail ?

[CosmoKnacki]

Je viens de voir ta regex mais je ne l'ai pas encore décortiquée, mais déjà je peux te dire:

• PHP dispose d'un tokenizer intégré muni de ces deux fonctions token_get_all et token_name qui d'après moi te simplifierai grandement la tâche.
• Quand tu construis une expression un peu complexe et un peu longue, n'hésite pas à te servir du modificateur x qui te permet d'ignorer les espaces dans la pattern et d'ajouter des commentaires en ligne, genre:

  Code :

  Sélectionner tout - Visualiser dans une fenêtre à part

  1
  2
  3
  4
  5
  ~
  babobibobu # youglounoute
  ( .*? ) [89]  # tournicoti
  (?=tournicoton)
  ~x

  ainsi que des groupes de capture nommés: (?<fruit>pomme). Ça simplifie la lecture et le débogage.

D'autre part regex101 t'indique 162289 steps, ce qui devrait être un signal pour dire que ta pattern est mal conçue. C'est beaucoup trop même pour une chaîne très longue.

Maintenant d'un point de vue plus théorique et général:
Si tu te retrouves dans la situation où le lookbehind (qui avec PCRE doit être fixe ou une alternative entre plusieurs sous-pattern fixes: (?<=babobi|babobibobu|ba)) te bloque et que \K ne suffit plus à te secourir, tu peux envisager de revoir ton approche et de construire une expression qui décrit la chaîne de bout en bout à l'aide éventuellement de l'ancre \G qui correspond à la position suivant le précédant match et qui par conséquent force la contiguïté des matchs de manière à ne rien laisser au hasard (pas de trous). Les patterns avec \G ne sont pas facile à construire au début (il faut de l'entrainement).

Quoiqu'il en soit, regarde d'abord du coté du tokenizer.

[CosmoKnacki]

En vrac:

Pour décrire du texte entre quote en ignorant les quotes échappées, la pattern est:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

'[^'\\]*+(?s:\\.[^'\\]*)*+'

Ce qui donne par exemple dans une chaîne en PHP (pour bien voir les échappements nécessaires):

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

$pattern = "~'[^'\\\\]*+(?s:\\\\.[^'\\\\]*)*+'~";

Il faut, autant que faire ce peut, éviter de commencer une pattern part une alternative, car dans le pire des cas toutes les positions de la chaîne doivent être testées avec chaque branche de ton alternative. Dans l'idéal une pattern doit commencer avec un caractère unique, car dans ce cas les positions intéressantes de la chaîne sont localisée par un algorithme de recherche rapide qui intervient avant même la marche du moteur de regex (c'est une optimisation de PCRE). À défaut, tu peux, quand c'est possible, utiliser la technique de "la discrimination par le premier caractère", c'est à dire, au lieu d'écrire alpha|beta|gamma tu écris (?=[abg])(?:alpha|beta|gamma) pour éluder la plupart des positions qui ne sont pas intéressantes sans avoir à tester chacune des branches.

< n'est pas un caractère spécial, inutile de l'échapper. Les caractères spéciaux sont ( ) { [ | . ? * + ^ $ \, il n'y en a pas d'autres (hors des classes de caractères ou éventuellement tu dois échapper - suivant sa position, et exception faite du délimiteur de pattern que tu as choisi). Tu n'es même pas obligé d'échapper { lorsque ça ne prête pas à confusion avec le quantificateur {m,n} ou {m}.

Ne pas créer de groupes de capture ou de groupes non-capturant inutiles, ça facilite la lecture et ça économise un peu de mémoire.

Souvent, avoir recours à une conditionnelle (?(condition)vrai|faux) révèle un mauvais design de la pattern. La plupart du temps il y a une manière plus simple et plus efficace de faire.

[LPA__]

Sacrée regexp que la tienne.

Merci

Une question. Pourquoi ne te limites tu pas à contrôler les paramètres passés aux différents scripts ? Plutôt que de te lancer dans ce travail ?

Si tu veux dire que plutôt que de créer un outil automatique, je pourrais directement le faire à la main, effectivement. Vu le temps que j'y ai passé, il n'est pas impossible qu'au final j'y aurais gagné du temps. Mais il y'a quand même beaucoup de fichiers, parfois très très longs (7000 lignes ...). Et si j'arrive à faire un script qui simplifie grandement la tâche, je me dis qu'il pourrait aussi servir à d'autre.

Je viens de voir ta regex mais je ne l'ai pas encore décortiquée, mais déjà je peux te dire:

• PHP dispose d'un tokenizer intégré muni de ces deux fonctions token_get_all et token_name qui d'après moi te simplifierai grandement la tâche.

Ah oui très intéressant ! Je vais regarder ça plus en détail, mais ça pourrai effectivement simplifier grandement pas mal de mes problèmes (même si j'aimais bien le tout regex, je suis un peu tomber amoureux au fur et à mesure de mes essais avec x)).

• Quand tu construis une expression un peu complexe et un peu longue, n'hésite pas à te servir du modificateur x qui te permet d'ignorer les espaces dans la pattern et d'ajouter des commentaires en ligne [...]
  
  Ça simplifie la lecture et le débogage.

J'y ai pensé puis je me suis dit "Bah, c'est pour moi, et je comprend ce qui se passe, pas besoin". Mais c'est une erreur. Je le ferai pour ma prochaine version.

D'autre part regex101 t'indique 162289 steps, ce qui devrait être un signal pour dire que ta pattern est mal conçue. C'est beaucoup trop même pour une chaîne très longue.

Vu qu'on parle d'un fichier entier, ça ne me choquait pas. Et puis, comme c'est un script qui n'a pas a être appelé un grand nombre de fois, et seulement par moi, le temps ne me posait pas de problème.

Si tu te retrouves dans la situation où le lookbehind (qui avec PCRE doit être fixe ou une alternative entre plusieurs sous-pattern fixes: (?<=babobi|babobibobu|ba)) te bloque et que \K ne suffit plus à te secourir, tu peux envisager de revoir ton approche et de construire une expression qui décrit la chaîne de bout en bout à l'aide éventuellement de l'ancre \G qui correspond à la position suivant le précédant match et qui par conséquent force la contiguïté des matchs de manière à ne rien laisser au hasard (pas de trous). Les patterns avec \G ne sont pas facile à construire au début (il faut de l'entrainement).

Ce n'est pas exactement la même chose que d'avoir le modificateur A ? Parce que c'est que fait la regex pour le moment. Mais il y'a beau ne pas avoir de trous, impossible de savoir si au moment de reprendre, on est ou non dans entre deux "".

Pour décrire du texte entre quote en ignorant les quotes échappées, la pattern est:'[^'\\]*+(?s:\\.[^'\\]*)*+'Ce qui donne par exemple dans une chaîne en PHP (pour bien voir les échappements nécessaires):$pattern = "~'[^'\\\\]*+(?s:\\\\.[^'\\\\]*)*+'~";

Ma version marche aussi, mais la tienne est plus rapide. Mais j'ai deux questions. Pourquoi pour un groupe non capturant tu utilise (?s: au lieu de (?: ?
Ensuite, le quantificateur *+, c'est un groupe atomique ?

Il faut, autant que faire ce peut, éviter de commencer une pattern part une alternative, car dans le pire des cas toutes les positions de la chaîne doivent être testées avec chaque branche de ton alternative. Dans l'idéal une pattern doit commencer avec un caractère unique, car dans ce cas les positions intéressantes de la chaîne sont localisée par un algorithme de recherche rapide qui intervient avant même la marche du moteur de regex (c'est une optimisation de PCRE). À défaut, tu peux, quand c'est possible, utiliser la technique de "la discrimination par le premier caractère", c'est à dire, au lieu d'écrire alpha|beta|gamma tu écris (?=[abg])(?:alpha|beta|gamma) pour éluder la plupart des positions qui ne sont pas intéressantes sans avoir à tester chacune des branches.

Optimisation intéressante. Je vais tacher de m'en souvenir.

< n'est pas un caractère spécial, inutile de l'échapper.

J'ai beau le savoir, j'oublie encore.

Ne pas créer de groupes de capture ou de groupes non-capturant inutiles, ça facilite la lecture et ça économise un peu de mémoire.

J'en ai fait des inutiles ? Il me semblait pas pourtant ô_o Tu pourrai me dire lesquels ?

Souvent, avoir recours à une conditionnelle (?(condition)vrai|faux) révèle un mauvais design de la pattern. La plupart du temps il y a une manière plus simple et plus efficace de faire.

C'est noté. J’essaierai de trouver des alternatives.

[CosmoKnacki]

Bon que les choses soient claires. Pour éviter des attaques XSS, ce n'est pas au niveau de l'affichage que ça se règle mais au niveau des données saisies. C'est à dire:

• l'utilisateur saisie des données dans un formulaire (dans ce cas on peut déjà limiter ce qui est saisie du coté client avec html5 et javascript, en gardant bien à l'esprit que tout ça peut être contourné), ou bien les données sont obtenues d'une source extérieur quelconque.
• Avant même de faire quoi que ce soit de ces données, elle doivent être validées ou "sanitisées" coté serveur de manière à ne pas présenter ce risque d'attaque XSS (ou d'injection SQL).
• Les données sont soit stockées en BDD (à ce niveau on peut/doit aussi intervenir en utilisant des requêtes préparées), soit transmises à une autre page.

En aucun cas ce problème ne doit se régler à l'affichage, mais bien dés que les données arrivent coté serveur. C'est à ce moment qu'il faut intervenir, pas après.

Maintenant si tu veux faire des modifications du code PHP sur plusieurs fichiers qui contiennent du html, du javascript, des css et du PHP, à moins d'être masochiste, tu dois d'abord isoler le code PHP du reste. Y arriver à coup de regex est une entreprise plutôt ardue dans la mesure ou rien n'empêche d'écrire des choses complètement tordues mais bel et bien valides:<?php echo "toto ?>"; ?> ou encore d'utiliser "$echo" comme nom de variable. C'est pourquoi utiliser le tokenizer pour modifier du code PHP avec PHP semble la seule voie raisonnable.

Je comprend très bien qu'à titre d'exercice tu veuilles expérimenter les regex pour résoudre ce problème, c'est une manière de s'éprouver et de progresser quel qu'en soit le résultat, mais pour le moment et dans ce cas, ça ne constitue pas une solution viable, robuste et pertinente. En résumé cela doit rester du coté du "sandbox", pour s'entraîner.


Maintenant concernant l'aspect technique des regex:

Vu qu'on parle d'un fichier entier, ça ne me choquait pas. Et puis, comme c'est un script qui n'a pas a être appelé un grand nombre de fois, et seulement par moi, le temps ne me posait pas de problème.

Ce n'est pas le nombre de fois qui est à craindre, mais l'échelle. Plus de 100,000 étapes, ça veut dire qu'avec un fichier un peu plus gros ou plus compliqué, la limite de backtracking peut être facilement atteinte.

Ce n'est pas exactement la même chose que d'avoir le modificateur A ? Parce que c'est que fait la regex pour le moment. Mais il y'a beau ne pas avoir de trous, impossible de savoir si au moment de reprendre, on est ou non dans entre deux "".

L'utilisation de \G est plus souple à l'usage. D'ailleurs je t'enjoins à t'entraîner à construire une pattern qui trouvera toutes les variables entre double quotes et pas celles à l'extérieur. Essaie d'abord un cas simple comme "abc $var1 def $var2" ghi $var3 jkl "$var4 mno $var5" pqr $var6 puis ensuite essaie de compliquer un peu en ajoutant des antislashes. Utilise ce patron: (?:\G(?!\A)|AMORCE)CE_QUI_N_EST_PAS_UN_QUOTE_OU_UN_DOLLAR\K(?:VARIABLE|FIN_DE_LA_CHAINE(*SKIP)(*FAIL)).

Ma version marche aussi, mais la tienne est plus rapide.

Le problème avec ta version, c'est qu'elle ne peut pas savoir si un antislash avant un simple quote échappe le simple quote ou s'il s'agit d'un antislash littéral (lui même échappé par un autre antislash), par exemple elle ne devrait pas réussir avec les chaînes suivantes:

Code txt :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
'abc\'
 
'abc\\'def'
 
'abc\\\'
 
'abc\\\\'def'
 
etc.

Mais j'ai deux questions. Pourquoi pour un groupe non capturant tu utilise (?s: au lieu de (?: ?
Ensuite, le quantificateur *+, c'est un groupe atomique ?

Tu peux "allumer" ou "éteindre" dans la pattern même des modificateurs dits "en ligne" (inline modifier), par exemple (?s) actionnera le mode singleline et (?-s) l'éteindra. L'autre particularité de ces "switch"s est qu'ils s'appliquent uniquement dans le groupe dans lequel ils sont contenus. Par exemple ^(?:ab(?i)c)d$ réussira avec "abCd" mais pas avec "abCD". Bref, (?s:...) est juste la contraction de (?:(?s)...) (NB: on ne peut faire ça qu'avec les groupes non-capturant.)

*+ est un quantificateur possessif.

J'en ai fait des inutiles ? Il me semblait pas pourtant ô_o Tu pourrai me dire lesquels ?

ici: (?:(?<=\\)) et là: (?:[^.])*?

[LPA__]

Bon que les choses soient claires. Pour éviter des attaques XSS, ce n'est pas au niveau de l'affichage que ça se règle mais au niveau des données saisies. C'est à dire:

• l'utilisateur saisie des données dans un formulaire (dans ce cas on peut déjà limiter ce qui est saisie du coté client avec html5 et javascript, en gardant bien à l'esprit que tout ça peut être contourné), ou bien les données sont obtenues d'une source extérieur quelconque.

Les contrôles html5/javascript sont là uniquement pour le confort de l'utilisateur, justement parce qu'ils peuvent être contournés ils ne sont d'aucune utilités pour se protéger d'une attaque (il m'a fallut deux minutes pour choper une extension firefox qui permet d'éditer les données en POST d'une page).

• Avant même de faire quoi que ce soit de ces données, elle doivent être validées ou "sanitisées" coté serveur de manière à ne pas présenter ce risque d'attaque XSS (ou d'injection SQL).

Si certaines attaques xss peuvent être contrés par des protection anti injection SQL et vice versa, ce n'est pas le cas pour tout. Pour chaque attaque, un type de défense. Là je ne m'occupe que d'XSS, pas d'injection SQL (même si les injection SQL sont autant, si ce n'est plus, importantes à gérer, mais c'est pas ce qu'on me demande).

• Les données sont soit stockées en BDD (à ce niveau on peut/doit aussi intervenir en utilisant des requêtes préparées), soit transmises à une autre page.

En aucun cas ce problème ne doit se régler à l'affichage, mais bien dés que les données arrivent coté serveur. C'est à ce moment qu'il faut intervenir, pas après.

Là pour le coup, je ne suis pas d'accord. Tout ce que j'ai pu lire sur les attaques XSS parlent d'une protection des données en sorties :
php.developpez.com/faq/?page=securite#securite-failleXSS
venom630.free.fr/geo/tutz/securite_informatique/xss/
http://stackoverflow.com/questions/1...-with-html-php
Sans compter que les données internes ne sont pas forcément fiables (données provenant d'autres sources pas forcément sécurisés, reste d'attaques passées, etc ...). Du coup, peu importe ce qui se trouve dans la BDD, les utilisateurs ne pourront pas subir d'attaque XSS.

Maintenant si tu veux faire des modifications du code PHP sur plusieurs fichiers qui contiennent du html, du javascript, des css et du PHP, à moins d'être masochiste, tu dois d'abord isoler le code PHP du reste. Y arriver à coup de regex est une entreprise plutôt ardue dans la mesure ou rien n'empêche d'écrire des choses complètement tordues mais bel et bien valides:<?php echo "toto ?>"; ?> ou encore d'utiliser "$echo" comme nom de variable. C'est pourquoi utiliser le tokenizer pour modifier du code PHP avec PHP semble la seule voie raisonnable.

Sauf que ce genre de truc complètement tordu, se trouve dans des balises php, donc, on peut les trouver et les sortir (et j'ai traités pas mal de cas tordu d'ailleurs dans une précédente version).Mais oui, si par raisonnable tu entend "carrément plus simple et sur" je pense que tu as raison.

Le problème avec ta version, c'est qu'elle ne peut pas savoir si un antislash avant un simple quote échappe le simple quote ou s'il s'agit d'un antislash littéral (lui même échappé par un autre antislash)

Ah oui bien vu ! J'avais jamais envisagé ce cas de figure ...


Pour les groupes inutiles, va falloir que je sois plus attentif

Et pour l'exercice, je vais m'y atteler dès que possible merci

[LPA__]

L'utilisation de \G est plus souple à l'usage. D'ailleurs je t'enjoins à t'entraîner à construire une pattern qui trouvera toutes les variables entre double quotes et pas celles à l'extérieur. Essaie d'abord un cas simple comme "abc $var1 def $var2" ghi $var3 jkl "$var4 mno $var5" pqr $var6 puis ensuite essaie de compliquer un peu en ajoutant des antislashes. Utilise ce patron: (?:\G(?!\A)|AMORCE)CE_QUI_N_EST_PAS_UN_QUOTE_OU_UN_DOLLAR\K(?:VARIABLE|FIN_DE_LA_CHAINE(*SKIP)(*FAIL)).

J'ai réussi : (?:\G(?!\A)|")[^"$]*\K(?:\$\w*|"(*SKIP)(*FAIL))Néanmoins, il y'a quelque chose que je ne comprend pas bien.
A quel moment le (?!\A) marche ou pas ? Quand on recommence après avoir matché (genre juste après $var1), il passe, mais si il y'a eu un échec (après le second "), il ne passe pas.