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

Potential XSS vulnerability in link extension #3673

Closed
1 of 2 tasks
fdoe opened this issue Jan 30, 2023 · 9 comments
Closed
1 of 2 tasks

Potential XSS vulnerability in link extension #3673

fdoe opened this issue Jan 30, 2023 · 9 comments
Labels
Type: Bug The issue or pullrequest is related to a bug

Comments

@fdoe
Copy link

fdoe commented Jan 30, 2023

What’s the bug you are facing?

It is possible to execute JavaScript from the "href" attribute of a link.

Which browser was this experienced in? Are any special extensions installed?

Chrome 109.0.5414.119
Firefox 109.0
Safari 16.2
Edge 109.0.1518.70

How can we reproduce the bug on our side?

  1. Open code sandbox: https://codesandbox.io/s/tiptap-react-issue-template-nwvwck?file=/src/App.js
  2. Replace content of App.js with:
import React from "react";
import { useEditor, EditorContent } from "@tiptap/react";
import {Link} from '@tiptap/extension-link';
import StarterKit from "@tiptap/starter-kit";

export default () => {
  const editor = useEditor({
    extensions: [
      StarterKit,
      Link.configure({
        validate: href => /^https?:\/\//.test(href),
      })
    ],
    content: {
      "type": "doc",
      "content": [
        {
          "type": "paragraph",
          "content": [
            {
              "type": "text",
              "marks": [
                {
                  "type": "link",
                  "attrs": {
                    "href": "javascript:alert(1)",
                    "target": "_blank",
                    "class": null
                  }
                }
              ],
              "text": "XSS attack link"
            }
          ]
        }
      ]
    }
  });

  return (
    <>
      <EditorContent editor={editor} />
    </>
  );
};
  1. Click link displayed by the editor and wait for the JavaScript to open an alert popup

Can you provide a CodeSandbox?

Yes. See "How can we reproduce the bug on our side?".

What did you expect to happen?

Ideally, the editor would validate / sanitize the href attribute before rendering.

Anything to add? (optional)

Currently, the JSON content representation of the editor must be validated / sanitized separately in order to clear it from any malicious JavaScript. While this should be could practice anyways, the editor should perform such a sanitization by itself, too, as a safety measure.

Did you update your dependencies?

  • Yes, I’ve updated my dependencies to use the latest version of all packages.

Are you sponsoring us?

  • Yes, I’m a sponsor. 💖
@fdoe fdoe added the Type: Bug The issue or pullrequest is related to a bug label Jan 30, 2023
@bdbch bdbch added this to the 2.0.0 milestone Jan 30, 2023
@bdbch
Copy link
Contributor

bdbch commented Jan 30, 2023

I think this is related to #2169 which includes a more lengthy discussion about this topic. I'll keep this open for discussion and to keep track of what we want to do here.

@fdoe
Copy link
Author

fdoe commented Jan 30, 2023

Thank you @bdbch for the quick reaction and the reference to #2169 !

@fdoe
Copy link
Author

fdoe commented Feb 9, 2023

Update:

Apparently, TipTap treats JSON and HTML content differently:

  1. Open code sandbox: https://codesandbox.io/s/tiptap-react-issue-template-nwvwck?file=/src/App.js
  2. Replace content of App.js with:
import React from "react";
import { useEditor, EditorContent } from "@tiptap/react";
import StarterKit from "@tiptap/starter-kit";
import {Link} from '@tiptap/extension-link';
 
export default () => {
  const editor = useEditor({
    extensions: [
      StarterKit,
      Link.configure({
        validate: href => /^https?:\/\//.test(href),
      })
    ],
    content: `
      <a href="javascript:alert(1)">Sanitized XSS attack link</a>
    `
  });
 
  return (
    <>
      <EditorContent editor={editor} />
    </>
  );
};
  1. Im comparison to the XSS attack example before, TipTap is used with HTML content rather than JSON content. This time, TipTap is able to sanitize the link properly by removing the malicious "href" attribute and converting the <a> tag to a <p> tag.

This is particularly concerning, as the TipTap security documentation explicitly states "There is no reason to use one or the other [format] because of security concerns".

Either the documentation should be updated and clearly point out to the different treatment of JSON and HTML or the sanitization suggestions in #2169 should be implemented as default behavior.

@bdbch bdbch removed this from the 2.0.0 milestone Feb 23, 2023
@github-actions
Copy link
Contributor

This issue is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the Info: Stale The issue or pullrequest has not been updated in a while and might be stale label May 25, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 1, 2023
@bdbch bdbch reopened this Jun 1, 2023
@github-actions github-actions bot removed the Info: Stale The issue or pullrequest has not been updated in a while and might be stale label Jun 2, 2023
@jhrncar
Copy link

jhrncar commented Jun 28, 2023

any updates on this?

@tomo-kn
Copy link

tomo-kn commented Dec 19, 2023

It seems to have already been resolved here: #4602

@Nantris
Copy link
Contributor

Nantris commented Dec 19, 2023

I'm probably not aware of every method of inserting XSS into href, but I wasn't able to produce any dangerous links in my testing - at least in Chrome.

I also tried this approach, which doesn't obviously include the word javascript but it still seems to be filtered out: <a href="&#x58;&#x61;&#x76;&#x61;&#x73;&#x63;&#x72;&#x69;&#x70;&#x74;&#x3A;&#x61;&#x6C;&#x65;&#x72;&#x74;&#x28;&#x27;&#x58;&#x53;&#x53;&#x40;&#x27;&#x29;">dangerous</a> and it was not presented as a link.

https://codesandbox.io/p/sandbox/unruffled-napier-9k5ns3


That said, the maintainers should give this another look before this issue is closed.

@julmud
Copy link

julmud commented May 15, 2024

It seems to have already been resolved here: #4602

Yes but no... ;-) The sanitization of the protocol seems to be done case sensitively when the output is JSON. Consequently, on the sandbox proposed in the original message, changing "javascript:alert(1)" to "Javascript:alert(1)" still allows the XSS attack to succeed...

Fun fact: if the format is HTML instead of JSON, the sanitization works whether the "javascript:" is in lower case or not.

@Nantris
Copy link
Contributor

Nantris commented May 16, 2024

@chroth implemented a whitelist system which should address this. I invite everyone to review it: #5160

Unless anyone raises any issues, I think this issue can truly be closed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug The issue or pullrequest is related to a bug
Projects
Status: Done
Development

No branches or pull requests

7 participants