Skip to content

Remove require htmlLawed from Lib folder - #49

Merged
PHPierrre merged 4 commits into
MineWeb:masterfrom
StanByes:patch-1
Dec 13, 2021
Merged

Remove require htmlLawed from Lib folder#49
PHPierrre merged 4 commits into
MineWeb:masterfrom
StanByes:patch-1

Conversation

@StanByes

Copy link
Copy Markdown

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
Copy Markdown
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

nivcoo commented Dec 11, 2021

Copy link
Copy Markdown
Member

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

nivcoo commented Dec 11, 2021

Copy link
Copy Markdown
Member

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
Copy Markdown
Author

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

@nivcoo

nivcoo commented Dec 12, 2021

Copy link
Copy Markdown
Member

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

@StanByes

Copy link
Copy Markdown
Author

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

@PHPierrre

Copy link
Copy Markdown
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

nivcoo commented Dec 13, 2021

Copy link
Copy Markdown
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.

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

@nivcoo

nivcoo commented Dec 13, 2021

Copy link
Copy Markdown
Member

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

Comment thread Controller/ForumController.php Outdated
if ($this->request->is('post')) {

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

Copy link
Copy Markdown
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
Copy Markdown
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

nivcoo commented Dec 13, 2021

Copy link
Copy Markdown
Member

@StanByes Tu as oublié une parenthèse

@nivcoo

nivcoo commented Dec 13, 2021

Copy link
Copy Markdown
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 ?

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
Copy Markdown
Author

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

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

nivcoo commented Dec 13, 2021

Copy link
Copy Markdown
Member

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

@nivcoo

nivcoo commented Dec 13, 2021

Copy link
Copy Markdown
Member

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

@PHPierrre

Copy link
Copy Markdown
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