Project

General

Profile

Anomalie #4706

safehtml vire des trucs légitimes

Added by RastaPopoulos ♥ 3 months ago. Updated about 1 month ago.

Status:
Fermé
Priority:
Normal
Assignee:
-
Category:
-
Target version:
Start date:
03/29/2021
Due date:
% Done:

0%

Resolution:
fixed
Navigateur:

Description

Il semblerait que safehtml fasse quelque peu de la merde : ça vire des attributs totalement légitimes, par ex des data-truc où on met ce qu'on veut.
En 3.2 ou 3.3, à corriger dans les deux.

Par exemple le plugin Intl qui a une fonction d'affichage des montants, utilise le mot "montant" dans des attributs (logique vu que c'est tout l'objet du plugin).

J'ai bien testé avec juste charger_fonction('safehtml', 'inc') de ce plugin.

$safehtml('<span class="montant" data-montant-nombre="100" data-montant-devise="EUR">')

Au retour ya plus que :

<span class="montant" 

Et du coup ça affiche l'emoji danger avec le code visible, par exemple dans des affichages du plugin Bank, mais pas que, dès qu'on passe ce HTML à une chaine de langue un truc comme ça.

History

#1 Updated by RastaPopoulos ♥ 3 months ago

  • Description updated (diff)

#2 Updated by marcimat 🌻 3 months ago

Plutôt que de corriger cette lib qui vieillit, il faudrait passer à html purifier ...
https://core.spip.net/issues/3926

#3 Updated by RastaPopoulos ♥ 3 months ago

Ça sera sûrement mieux oui, par contre j'ai mis 3.2 car ça va resté maintenu et stable un moment quand même (même après 3.3 sorti), et donc tous les gens qui ont un site avec de la vente (ou des dons) et qui sont en 3.2, s'ils mettent juste à jour leurs plugins régulièrement, bah ils vont tous avoir certains affichages pétés (puisque désormais Prix/Intl génère ce HTML avec des data persos, et que cet affichage de montant peut être passé à des chaines de langue, comme dans Bank notamment mais pas que).

#4 Updated by RastaPopoulos ♥ 3 months ago

Pour résoudre sans changer de lib (donc y compris 3.2), il faudrait modifier la lib safehtml() pour que ça ne supprime au moins aucun data-truc qui sont dans tous les cas totalement légitimes. La lib a été faite à un moment où ça n'existait (fort longtemps donc), mais maintenant il y en a un peu partout des data-truc.

#5 Updated by Brice Boucard 2 months ago

Bonjour,

Je me permets de faire un lien vers une observation du ticket concernant l'utilisation de la balise < pre > avec les attributs data- qui disparaissent (https://core.spip.net/issues/3990#note-8).

#6 Updated by marcimat 🌻 2 months ago

J'ai du mal à me faire un avis sur ces attributs data-.

Dans ton cas Rasta, le code ajouté ne provient pas d'une saisie utilisateur, pourquoi alors passer par safehtml() ?
Est-ce qu'on a pas moyen de dire que "je sais que mon code html que j'ajoute est valide / ok" : c'est le cas si on ajoute des modèles il me semble.

Mais si on considère que les attributs data- sont utilisés derrière en JS pour charger des trucs divers s'ils sont présents, c'est peut être pas faux de les brider dans les saisies utilisateurs…
Bref, tout ça me questionne !

#7 Updated by RastaPopoulos ♥ 2 months ago

Mais moi je passe nulle part, c'est les chaines de langue qui y passent automatiquement (sanitize=true par défaut).

Mais quand même ça n'y passe que pour les arguments. Sauf que là le HTML c'est dans une chaine de langue du plugin Intl certes, mais qui ensuite est inséré en argument dans une chaine de langue du plugin Bank, de mémoire. Genre une phrase qui inclut un montant formaté dedans (#MONTANT|montant_formater en gros).

En attendant il faudrait au moins que Bank ajoute un sanitize=false sur cette appel, un truc comme ça.

#8 Updated by RealET 🔸 about 2 months ago

Autre cas problématique : la prévisualisation des slides avec nivoslider : https://contrib.spip.net/Nivo-Slider-3747#comment508245-508242

#9 Updated by cedric - about 1 month ago

ça vaut ce que ça vaut, mais j'ai mis la lib a jour et introduit des tests unitaires
https://git.spip.net/spip/safehtml/commits/branch/master

#10 Updated by cedric - about 1 month ago

  • Status changed from Nouveau to Fermé
  • Target version changed from 3.2 to 4.0
  • Resolution set to fixed

Et avec les tests unitaires, tout va mieux : c'est donc corrigé en 4.0-dev par https://git.spip.net/spip/safehtml/commit/ad3f445ec89181bafd31ed82726810b17f77bd91
(je sais pas si il faut reporter en 3.2, je suis pas certain certain)

Et par contre RealET, ton url en data:xxx est un cas légitime et potentiellement dangereux, il est donc normal que safehlml la supprime, ne t'en déplaise

#11 Updated by RastaPopoulos ♥ about 1 month ago

excellent merci !

#12 Updated by RealET 🔸 about 1 month ago

cedric - a écrit :

Et par contre RealET, ton url en data:xxx est un cas légitime et potentiellement dangereux, il est donc normal que safehlml la supprime, ne t'en déplaise

Super, merci.

Et pour NivoSlider, c'est résolu : https://git.spip.net/spip-contrib-extensions/nivoslider/commit/2fc16f04

Also available in: Atom PDF