Strip tags to sanitize input in User profile

[gautiermichelin]

Strip tags to sanitize input in User profile and in Tags & Comments.
I had a mail from etat.ge.ch audit firm that reports these as XSS injection risk in their automated exam.

[collectiveaccess]

Striptags() is not enough. HTMLPurifier must be used. I'll add that shortly.

[gautiermichelin]

Hi,

JavaScript is already removed (tested) but simple tags , ... are kept without attributes.

Gautier

[collectiveaccess]

What is the problem then?

[gautiermichelin]

Sorry, easier to understand : “Those tags are still displayed everywhere (dropdown user menu, bottom bar in Providence for user name, within the tags or comments…) ; and not tested, but tags may be cutted with a comma within some tags :-(“ Le ven. 17 juin 2022 à 14:19, Gautier Michelin ***@***.***> a écrit :

Hi Seth or CA team, Those tags are still displayed everywhere (dropdown user menu, bottom bar in Providence for user name, within the tags are commands…), and not tested, but tags may be cutted with a comma within some tags :-( For me, it’s more a question than etat.ge.ch their tool will continue to report false XSS injection risks (you’ll risk having such demand from other users too) why not simple strip_tags ? Best, Gautier Le ven. 17 juin 2022 à 14:13, CollectiveAccess ***@***.***> a écrit : > What is the problem then? > > — > Reply to this email directly, view it on GitHub > <#42 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AACM2SM6JYTMXLBHZGT36ELVPRTXRANCNFSM5ZBY3R2Q> > . > You are receiving this because you authored the thread.Message ID: > ***@***.***> > -- Gautier MICHELIN

-- Gautier MICHELIN

[collectiveaccess]

Is this an XSS risk? Or just a formatting issue?

[collectiveaccess]

I'm going to merge this, as regardless there's no reason for HTML tags to be in there.

[gautiermichelin]

Hi @collectiveaccess

I'm just answering your question.

For me, formatting isn't needed in these. As you can add tags here, it's reported as a risk with certains (at least one) automated XSS report tool. Let's remove this false positive.

Thanks for having merged this PR, best to all team member

[gautiermichelin]

Hi Seth or CA team, Those tags are still displayed everywhere (dropdown user menu, bottom bar in Providence for user name, within the tags are commands…), and not tested, but tags may be cutted with a comma within some tags :-( For me, it’s more a question than etat.ge.ch their tool will continue to report false XSS injection risks (you’ll risk having such demand from other users too) why not simple strip_tags ? Best, Gautier Le ven. 17 juin 2022 à 14:13, CollectiveAccess ***@***.***> a écrit :

What is the problem then? — Reply to this email directly, view it on GitHub <#42 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACM2SM6JYTMXLBHZGT36ELVPRTXRANCNFSM5ZBY3R2Q> . You are receiving this because you authored the thread.Message ID: ***@***.***>

-- Gautier MICHELIN

[collectiveaccess]

Ok sure let's do that then.

Can you make a new PR for both master and develop?

thanks