Skip to content

Conversation

@StanByes
Copy link

Mineweb inclut, depuis les dernières version, htmlLawed dès le AppController donc le plugin essayait de le require de nouveau ce qui provoquait une Internal Server Error

Mineweb inclut, depuis les dernières version, htmlLawed dès le AppController donc le plugin essayait de le require de nouveau ce qui provoquait une Internal Server Error
@PHPierrre
Copy link
Member

J'ai remarqué ca ici : #47
Voilà, maintenant, il faut supprimer les fichiers correspondants.

Autre problème : visiblement la librairie laisse passer des choses : #42

@nivcoo
Copy link
Member

nivcoo commented Dec 11, 2021

J'avais pas vu les issues/pull du plugin, enfaite quand tu l'as mis sur le forum les attributes étaient tous autorisés, hors on peut faire passer du xss dans des attributes html, donc normalement l'application sur toute les requetes du cms devrait corriger le problème, pour l'erreur 500 sur le forum je ne me suis pas pencher dessus même si je pense que certaines fonctionnalités peuvent être impactés par ces modif (Simplement au niveau de certain formulaire spécial je pense).

@nivcoo
Copy link
Member

nivcoo commented Dec 11, 2021

Cependant

return htmLawed($content, ['safe' => 1]);
n'a pas la même config que : https://github.com/MineWeb/MineWebCMS/blob/development/app/Controller/Component/EySecurityComponent.php#L33

Il faudrait supprimer tout les appels à la fonction "removeScript" puisque ce n'est plus nécessaire et ce serait de la duplication de protection

@StanByes
Copy link
Author

Je peux m'en charger si vous voulez. Ça va pas prendre 4 ans non plus

@nivcoo
Copy link
Member

nivcoo commented Dec 12, 2021

Nan techniquement c'est qu'un ctrl f mais tu peux le faire et le mettre dans ce pull

@StanByes
Copy link
Author

Normalement c'est bon avec les deux commits (le premier est un fail)

@PHPierrre
Copy link
Member

Normalement c'est bon avec les deux commits (le premier est un fail)

Le problème c'est que je ne peux pas distinguer ce qui a été modifié là.

Idéalement il faudrait garder le commit initial, retirer le fichier htmLawed, et peut être meme faire appel directement à celui du CMS afin d'optimiser les argument donnés à la fonction htmLawed.

@nivcoo
Copy link
Member

nivcoo commented Dec 13, 2021

Normalement c'est bon avec les deux commits (le premier est un fail)

Le problème c'est que je ne peux pas distinguer ce qui a été modifié là.

Idéalement il faudrait garder le commit initial, retirer le fichier htmLawed, et peut être meme faire appel directement à celui du CMS afin d'optimiser les argument donnés à la fonction htmLawed.

Il n'y a plus besoin de ça puisque toutes les requêtes du CMS passent par htmlawed

@nivcoo
Copy link
Member

nivcoo commented Dec 13, 2021

Pour distinguer les changements regarde directement dans l'onglet file changed, il n'y pas le problème d'indentation

if ($this->request->is('post')) {

$newMessage = $this->removeScript($this->request->data['content']);
$newMessage = $this->request->data['content']);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmmm )

@PHPierrre
Copy link
Member

Pour distinguer les changements regarde directement dans l'onglet file changed, il n'y pas le problème d'indentation

Bien vu !
Du coup le CMS effectue les vérifications nécessaire, plus besoin de se soucier de quelque chose ?

@nivcoo
Copy link
Member

nivcoo commented Dec 13, 2021

@StanByes Tu as oublié une parenthèse

@nivcoo
Copy link
Member

nivcoo commented Dec 13, 2021

Pour distinguer les changements regarde directement dans l'onglet file changed, il n'y pas le problème d'indentation

Bien vu ! Du coup le CMS effectue les vérifications nécessaire, plus besoin de se soucier de quelque chose ?

Oui tous les $this->request->data passe par un htmlawed avec les bons attribues directement dans le cms (Bien évidement pour certaines requêtes ça peut poser soucis mais le grand maximum a été corrigé)

@StanByes
Copy link
Author

C'est bon j'ai enlevé la paranthèse ;)

@PHPierrre PHPierrre merged commit 4d2813b into MineWeb:master Dec 13, 2021
@nivcoo
Copy link
Member

nivcoo commented Dec 13, 2021

@PHPierrre Up la version requise du cms pour le plugin à la dernière 1.15.2

@nivcoo
Copy link
Member

nivcoo commented Dec 13, 2021

Ou du moins 1.15.1 (là où a été appliqué le fix)

@PHPierrre
Copy link
Member

Merci à vous @StanByes et @nivcoo 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants