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

[BUG] data-ajax-replace strips away target attribute #9390

Open
ahmad-shahid opened this issue Jun 29, 2022 · 5 comments
Open

[BUG] data-ajax-replace strips away target attribute #9390

ahmad-shahid opened this issue Jun 29, 2022 · 5 comments
Assignees
Labels
Need: Use case The usefulness need to be more detailed. Defining what is the desired outcome. Query: Feature Request

Comments

@ahmad-shahid
Copy link

Hello, an issue has come up for CDTS. I was hoping to get your thoughts on this.

When using data-ajax-replace, if I'm inserting an anchor tag with a target attribute so that a link opens in a new window, it looks like the target attribute is removed when the data is inserted.

To Reproduce
Steps to reproduce the behaviour:

  1. Go to https://wet-boew.github.io/v4.0-ci/demos/data-ajax/data-ajax-en.html
  2. For the data-ajax-replace example, for the content that is inserted in, if you add for example <a href="https://google.ca", target="_blank">Link</a>, the content inserted will have a link but target="_blank" will not be there.

Expected behaviour
For <a> tags, the target attribute not be removed when inserted using data-ajax-replace.

Version of WET-BOEW/GCWeb you are using
This problem started happening in v4.0.44

@Christopher-O
Copy link
Contributor

#9313 discusses this issue.

@duboisp
Copy link
Member

duboisp commented Jul 6, 2022

@ahmad-shahid that is the expected behaviour since we applied security fix to jQuery through DomPurify. That change was documented into our release note of wet-boew.

This is not only applicable to data-ajax, but to any HTML code that get parsed through a jQuery command for wet-boew and any third party script that use jQuery.

However, @Christopher-O has submitted a request to Principal Publisher in order to support a valid use case of opening a new windows when the web author has applied additional security measure to prevent tabnabbing exploit.

@ahmad-shahid your proposed use case is currently not valid.

I suggest you to submit also a request to Principal Publisher regarding that.

\cc @GormFrank

@joejoseph00
Copy link
Member

yes 4.0.44.x causing us some kind of issue in this regard.

I'm highly skeptical of "security" "fixes" that break functionality unfortunately I don't have the time to investigate fully or to propose an alternative fix.

What's most broken about this is there's no exception, no console log, no warning about the offending behavior. What did we do wrong? Which CVE is offended? Who knows. As a developer we're supposed to dig dig dig dig dig and unravel someone elses decisions to even figure out why things stop working with a new release of wet-boew. In this case we often won't notice or find out until months after upgrading when someone complains. There are limits to automated test coverage.

Good friendly assistance would be to offer API breakages with a helpful log message pointing to the cve and explaining why we've offended someone or some thing , or better yet, to find ways to solve the cve without breaking functionality. Please throw us a few bones here. We're hungry dogs sludging it out in the trenches.

@duboisp
Copy link
Member

duboisp commented Dec 16, 2022

@joejoseph00 we have followed the recommendation as stated by the CVE which was to leverage DOMPurify as we noted in our release notes of v4.0.44.x

Here the security advisories:

Our solution of fixing jQuery v2 directly was chosen because it was the fastest and easiest way to quickly fix the security issue. A lot more of work and a lot more of testing would be required if we tried to move to jQuery 3. Although, we had a community member @neilmispelaar that started that migration project awhile ago (Research 2019-16) but it is still incomplete. You are welcome to help us.

offer API breakages

In this specific situation, that change is not consider as an "API breakages" which I interpret as major change" according to our versioning and our versioning public API which is currently under review with a lot more details


If you need immediate answer to a question or want to know how you can contribute to fix issue in wet-boew (and gcweb), you can join us at anytime during our weekly Tuesday afternoon office hours. You will find the MS Team meeting link in our wiki page.

@joejoseph00
Copy link
Member

joejoseph00 commented Dec 16, 2022

CVE-2020-11023
Undergoing Reanalysis

This vulnerability has been modified and is currently undergoing reanalysis. Please check back soon to view the updated vulnerability summary.

Perhaps someone is disputing this?

https://nvd.nist.gov/vuln/detail/cve-2020-11023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Need: Use case The usefulness need to be more detailed. Defining what is the desired outcome. Query: Feature Request
Projects
None yet
Development

No branches or pull requests

5 participants