Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reflected Cross-Site Scripting (XSS) Vulnerabilities #92

Closed
koentjeh opened this issue Aug 31, 2023 · 5 comments · Fixed by #127
Closed

Reflected Cross-Site Scripting (XSS) Vulnerabilities #92

koentjeh opened this issue Aug 31, 2023 · 5 comments · Fixed by #127

Comments

@koentjeh
Copy link

We found that some templates in the Tweakwise modules are vulnerable to XSS attacks.

Magento has strategies within the dev docs that could help prevent these attacks: https://devdocs.magento.com/guides/v2.3/extension-dev-guide/xss-protection.html

One found example is wrapping the $variables within $block->escapeHtml().

<input type="hidden" name="<?=$name?>" value="<?=$value?>">

@ah-net
Copy link
Collaborator

ah-net commented Sep 12, 2023

@koentjeh Dat is correct. But escaping all fields in that form leads to problems with urls in some rare cases. Thats why we've added the tw_hash field. This should prevent xss atacks in that form.

Do you still have xss issues with that form?

@arnoschoon
Copy link

Hi @ah-net ,

from the minimal information shared in #101 it seems to be related, yet neither of the issues mention any plan to remediate this issue.

Could you explain what makes "escaping all fields in that form" so difficult?

Maybe the cheat sheet from OWASP that provides guidance to prevent XSS vulnerabilities. could be of help.

@ah-net
Copy link
Collaborator

ah-net commented Nov 16, 2023

@arnoschoon

#101 is not related to this issue because the xss issue is in een different part. And that part is actually already escaped in the html.

I've not fixed this issue, because i'm not convinced it is still an issue here. There is an xss check for changed variables with an hash field on that form. I've you can give an example it's still an issue in the latest version than i will fix this issue. I've you don't want to share an example here you can send it to [email protected] and mention it's an xss issue in the magento plugin.

The problem was that escaping some url values caused the ajax request to fail or make some filters not selectable. I can't seem to reproduce this problem anymore (a lot of changes where made to the filtering in the meantime) but I'm not sure if the problem is fixed in all cases (depends on different settings)

@arnoschoon
Copy link

Hi @ah-net ,

thanks for your extensive explanation, good to hear this does not relate to #101 :)

At time of writing my colleague is working on a patch to block reflected XSS, the issue we got from a security scan performed on one of our shops. If applicable I'll try to share it with you if it's ready and working. In the meantime I'll forward the uncovered issue to [email protected] so you can determine if it really originates from this module.

@ah-net
Copy link
Collaborator

ah-net commented Nov 23, 2023

@arnoschoon Thx for the report. I can reproduce the xss issue locally. We will investigate the issue futher and see if it really originates in our module or if it's an theme issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants