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

Tabnabbing vulnerability in snow theme #2438

Closed
jonathanlloyd opened this issue Dec 21, 2018 · 5 comments
Closed

Tabnabbing vulnerability in snow theme #2438

jonathanlloyd opened this issue Dec 21, 2018 · 5 comments

Comments

@jonathanlloyd
Copy link
Contributor

Steps for Reproduction

  1. Visit https://quilljs.com/standalone/snow/
  2. Create a link to some url
  3. Click the link to get the link preview
  4. Inspect the link element

Expected behavior:
The link has the target attribute set to _blank but has no rel property. This means that documents containing untrusted links make the page they are embedded in susceptible to tabnabbing https://www.owasp.org/index.php/Reverse_Tabnabbing.

It would be expected that the rel property be set to noopener (possibly also norefferer and nofollow)

Actual behavior:
No rel property is set

Platforms:
All
Include browser, operating system and respective versions

Version:
All
Run Quill.version to find out

@jonathanlloyd
Copy link
Contributor Author

The issue is in quill/themes/snow.js line 72-77:

SnowTooltip.TEMPLATE = [
  '<a class="ql-preview" target="_blank" href="about:blank"></a>',
  '<input type="text" data-formula="e=mc^2" data-link="https://quilljs.com" data-video="Embed URL">',
  '<a class="ql-action"></a>',
  '<a class="ql-remove"></a>',
].join('');

@d4l-w4r
Copy link
Contributor

d4l-w4r commented Jul 4, 2019

Hey @jhchen !
I just stumbled over this vulnerability while auditing our app.
The fix is merged but there doesn't seem to have been a deployment since then.

Do you have any timeline to release a bugfix version soon? Or could you create a new patch tag 1.3.7 at the mitigating commit (aceaf9f) and release a patch update?

As it is right now, this issue probably shouldn't be closed because it still requires action by the maintainer before the vulnerability is actually fixed for users.

@d4l-w4r
Copy link
Contributor

d4l-w4r commented Jul 6, 2019

Hi!
I found that Jonathans fix does not completely protect quill users from tabnabbing.
I have added PR #2674 to add the rel="noopener noreferrer" attribute to anchor tags created by formats/link.js

cdesch added a commit to cdesch/django-quill-editor that referenced this issue Feb 5, 2021
Bump to Quill Version 1.3.7 - This version of Quill fixes Quill Vuln quilljs/quill#2438

Here is the change commit to fix the vuln in Quill quilljs/quill#2439

The Vuln is described here: https://ossindex.sonatype.org/vuln/d96c07dd-81f9-41f6-b2bd-531143bcaeab
LeeHanYeong pushed a commit to LeeHanYeong/django-quill-editor that referenced this issue Feb 9, 2021
* Bump to Quill Version 1.3.7

Bump to Quill Version 1.3.7 - This version of Quill fixes Quill Vuln quilljs/quill#2438

Here is the change commit to fix the vuln in Quill quilljs/quill#2439

The Vuln is described here: https://ossindex.sonatype.org/vuln/d96c07dd-81f9-41f6-b2bd-531143bcaeab

* Adding JS/CSS include instructions from README.md

Resolves issue [#33](#33)
LeeHanYeong pushed a commit to LeeHanYeong/django-quill-editor that referenced this issue Feb 27, 2021
…son_string' (#41)

* Bump to Quill Version 1.3.7

Bump to Quill Version 1.3.7 - This version of Quill fixes Quill Vuln quilljs/quill#2438

Here is the change commit to fix the vuln in Quill quilljs/quill#2439

The Vuln is described here: https://ossindex.sonatype.org/vuln/d96c07dd-81f9-41f6-b2bd-531143bcaeab

* Adding JS/CSS include instructions from README.md

Resolves issue [#33](#33)

* adding None return for json_string
@judaproteam
Copy link

judaproteam commented May 7, 2021

The issue is in quill/themes/snow.js line 72-77:

SnowTooltip.TEMPLATE = [
  '<a class="ql-preview" target="_blank" href="about:blank"></a>',
  '<input type="text" data-formula="e=mc^2" data-link="https://quilljs.com" data-video="Embed URL">',
  '<a class="ql-action"></a>',
  '<a class="ql-remove"></a>',
].join('');

why can i you just add to the code rel='noopene norefferer nofollow'
like this

<a class="ql-preview" target="_blank" href="about:blank" rel='noopene norefferer nofollow'></a>

i actuly try that, but it didnt change anything, why is that?

@d4l-w4r
Copy link
Contributor

d4l-w4r commented May 11, 2021

<a class="ql-preview" target="_blank" href="about:blank" rel='noopene norefferer nofollow'></a>

i actuly try that, but it didnt change anything, why is that?

This was a long time ago and I'm not sure if that TEMPLATE variable is the correct place to put the fix...but assuming it is, and that this is the exact code you wrote, then it's because of 2 typos:

  1. Your 'noopene' is missing an 'r'
  2. 'norefferer' is misspelled. It should be 'noreferrer'

So your code should look like this:
<a class="ql-preview" target="_blank" href="about:blank" rel='noopener noreferrer nofollow'></a>

LeeHanYeong pushed a commit to LeeHanYeong/django-quill-editor that referenced this issue Dec 8, 2021
* Bump to Quill Version 1.3.7

Bump to Quill Version 1.3.7 - This version of Quill fixes Quill Vuln quilljs/quill#2438

Here is the change commit to fix the vuln in Quill quilljs/quill#2439

The Vuln is described here: https://ossindex.sonatype.org/vuln/d96c07dd-81f9-41f6-b2bd-531143bcaeab

* Adding JS/CSS include instructions from README.md

Resolves issue [#33](#33)
LeeHanYeong pushed a commit to LeeHanYeong/django-quill-editor that referenced this issue Dec 8, 2021
…son_string' (#41)

* Bump to Quill Version 1.3.7

Bump to Quill Version 1.3.7 - This version of Quill fixes Quill Vuln quilljs/quill#2438

Here is the change commit to fix the vuln in Quill quilljs/quill#2439

The Vuln is described here: https://ossindex.sonatype.org/vuln/d96c07dd-81f9-41f6-b2bd-531143bcaeab

* Adding JS/CSS include instructions from README.md

Resolves issue [#33](#33)

* adding None return for json_string
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

No branches or pull requests

3 participants