Security Fix for Cross Site Scripting - huntr.dev #3782

huntr-helper · 2020-06-15T15:58:36Z

https://huntr.dev/app/users/Asjidkalam has fixed the Cross Site Scripting vulnerability 🔨. Asjidkalam has been awarded $25 for fixing the vulnerability through the huntr bug bounty program 💵. Think you could fix a vulnerability like this?

Get involved at https://huntr.dev/

User Comments:

📊 Metadata *

Please enter the direct URL for this bounty on huntr.dev. This is compulsory and will help us process your bounty submission quicker.

Bounty URL: https://www.huntr.dev/bounties/1-npm-summernote

⚙️ Abstract

Affected versions of this package are vulnerable to Cross-site Scripting (XSS). It is possible to inject JavaScript with object decoding such as <script>alert(1)</script> resulting in XSS. The fix is to sanitize the input on the package before further processing/executing.

❓ Technical description

Issue: #3719
From the issue, the package doesn't sanitize any HTML entities thrown at it. It is unsafe and causes reflected XSS in all cases. However, filtering out the tags which causes XSS from the DOM input can mitigate the issue.
The Fix is implemented in the dom_value function, which returns the input value (val) from the DOM. val variable is sanitized by replacing tags like &, >, < with &, < and > respectively. So i made a callback prototype function to achieve this:

String.prototype.escape = function() {
    var tagsToReplace = {
        '&': '&amp;',
        '<': '&lt;',
        '>': '&gt;'
    };
    return this.replace(/[&<>]/g, function(tag) {
        return tagsToReplace[tag] || tag;
    });
};

🐛 Proof of Vulnerability (PoV)

Open the editor, go to the Code View section and enter the following payload:
<script>alert(1)</script>, now deactivate the Code View to trigger the XSS.

Before patch:

🔥 Proof of Fix (PoF)

After patch:

It successfully sanitizes the input and deactivates from the Code View without triggering XSS.
The debug messages in the console shows the actual input and filtered input.

index.html file used in the POC:

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <script type="text/javascript" src="http://cdnjs.cloudflare.com/ajax/libs/jquery/3.4.1/jquery.min.js"></script>
    <link rel="stylesheet" href="http://cdnjs.cloudflare.com/ajax/libs/twitter-bootstrap/3.4.1/css/bootstrap.min.css" />
    <script type="text/javascript" src="http://cdnjs.cloudflare.com/ajax/libs/twitter-bootstrap/3.4.1/js/bootstrap.min.js"></script>

    <link href="summernote.css" rel="stylesheet">
    <script src="summernote.js"></script>
    
    <title>Summernote - XSS Fix POC</title>
</head>
<body>
    <div id="summernote">Hello summernote</div>
    <script>
        $(document).ready(function() {
            $('#summernote').summernote();
        });
    </script>
</body>
</html>

Test

Fixed Cross Site Scripting (XSS) in editor

dist/summernote.js

R2116 · 2020-06-29T19:43:11Z

Switching between the regular editor and the Code View back and forth escapes the Code View again and again for me:
All three tags to replace include an & which is one of the tags to replace - the & gets endlessly escaped.
Removing the first line of the tagsToReplace array solves it; but I'm sure it has some use?

Added a different regex on summernote.js to fix the issue

dist/summernote-lite.js

dist/summernote-bs4.js

Co-authored-by: R2116 <15418932+R2116@users.noreply.github.com>

JamieSlome · 2020-06-30T10:36:43Z

@R2116 - the requested changes have been made!

Thanks! 🍰

Augustukas · 2020-10-19T05:22:14Z

@lqez @easylogic @hackerwins bump

JamieSlome · 2020-10-26T16:49:37Z

@R2116 @lqez @easylogic @hackerwins - any updates on this?

Cheers! 🍰

DennisSuitters · 2020-11-26T15:04:29Z

@R2116 The changes to fix this vulnerability have been made to the wrong files, please do NOT edit the files in the dist/ folder, but rather the relevant files within the src/ folder, thank you.

DennisSuitters · 2020-11-30T02:17:48Z

I've just been reviewing this patch, there is an issue where the > and < in element tags are being converted, so for e.g. images are being displayed as <img src="image.jpg"> in the visual editor, but show with the entities in code view. We need to filter out elements so they are not affected by the patch. I'll have more of a play here in the meantime, but can you please update this to reflect the changes in the src/ folder, not the dist/ folder, thank you.
@R2116

DennisSuitters · 2020-11-30T03:16:52Z

Um, I remember in the settings there's options to filter the codeview data, which has options that this PR is trying to fix, and works.

    codeviewFilter: false,
    codeviewFilterRegex: /<\/*(?:applet|b(?:ase|gsound|link)|embed|frame(?:set)?|ilayer|l(?:ayer|ink)|meta|object|s(?:cript|tyle)|t(?:itle|extarea)|xml)[^>]*?>/gi,
    codeviewIframeFilter: true,
    codeviewIframeWhitelistSrc: [],
    codeviewIframeWhitelistSrcBase: [
      'www.youtube.com',
      'www.youtube-nocookie.com',
      'www.facebook.com',
      'vine.co',
      'instagram.com',
      'player.vimeo.com',
      'www.dailymotion.com',
      'player.youku.com',
      'v.qq.com',
    ],

DennisSuitters · 2021-01-06T05:06:20Z

@ivanovych666 I agree, except for the XSS issues while using the editor in the Browser, sanitising data before editing and database/other storage should take place.

Similar options like the codeview options should be implemented, I could do some changes as an experiment and see how that performs, I am mainly waiting through the discussion to determine what our best options for this are.

DennisSuitters · 2021-01-06T05:07:35Z

Looking at the changes in this PR, it looks like another PR should probably be made to reflect the changes without the dist/ file edits. We can continue to discuss best options or new ideas to accomplish this here for the time being.

nuclearghost · 2021-01-28T15:41:26Z

I'm actively dealing with a XSS attack in our app using summernote. I'd like to ensure this test case is covered for pasting into code view: '"><img src=x onerror=alert(document.domain)>. This doesn't hit any of our code before it renders, so not sure how best to insert a fix. I've tried enabling codeviewFilter: true, but it did not resolve this.

ivanovych666 · 2021-01-29T11:41:15Z

alert(document.domain)

@nuclearghost you can insert this into dev tools console and you will have the same result. This is not an issue. The issue if this script can be executed for another user. But this is not a WYSIWYG editor issue, it is up to developer to properly escape values before displaying to users or storring somewhere. BTW, we use this editor on a project and successfully passed a professional security audit without any issues. So it is up to you of how to use it. But I'm agree, that the ability to filter values immediatelly will be an awesome feature.

nuclearghost · 2021-01-29T18:43:21Z

Thanks for the feedback @ivanovych666 . We're probably outside the scope for this PR, but this seems to be the active XSS thread.

I understand there are different type of attacks for XSS. There's stored XSS where I totally agree it's not the responsibility of the WYSIWYG editor. I'm also concerned about the case where a non-technical user pastes in some HTML into the code view and then views it in the normal editor mode. I'll attempt to add a callback where this can be handled in a separate PR.

MrGoatsy · 2021-02-06T09:25:19Z

I managed to somewhat fix it by having my code like this:

<textarea name="description" id="description"></textarea>
<script>
    $('#description').summernote({
        height: 250,
        codeviewFilter: false,
        codeviewIframeFilter: true,
        // toolbar
        toolbar: [
            ['font', ['bold', 'italic', 'underline', 'clear']],
            ['color', ['color']],
            ['para', ['ul', 'ol', 'paragraph']],
            ['view', ['fullscreen', 'codeview', 'help']]
        ],
    }).on("summernote.enter", function(we, e) {
        $(this).summernote('pasteHTML', '<br />&VeryThinSpace;');
        e.preventDefault();
    });
    $("#description").summernote("code", "<p>&lt;script&gt;alert('123');&lt;/script&gt;<br /></p>");
</script>

This still doesn't fix it if you try type '".

PHP Tidy might be worth looking into:
https://www.php.net/tidy

I found a solution by using addslashes($string) with PHP, so this would be the code:

<?php
    $string = <<<EOD
        '"
    EOD;
?>
$("#description").summernote("code", "<?php echo addslashes($string); ?>");

The $string can also be fetched information from a database.

The result:

DennisSuitters · 2021-02-06T10:37:52Z

I think we need a solution that doesn't require external libraries, we need to consider that not everyone uses PHP.

MrGoatsy · 2021-02-06T10:52:39Z

I think we need a solution that doesn't require external libraries, we need to consider that not everyone uses PHP.

With Javascript you can also use a heredoc like this:

<textarea name="description" id="description"></textarea>
<script>
   var noXSS = (function() {/*This works
'"`
    */}).toString().match(/[^]*\/\*([^]*)\*\/\}$/)[1];

    $('#description').summernote({
        height: 250,
        codeviewFilter: false,
        codeviewIframeFilter: true,
        // toolbar
        toolbar: [
            ['font', ['bold', 'italic', 'underline', 'clear']],
            ['color', ['color']],
            ['para', ['ul', 'ol', 'paragraph']],
            ['view', ['fullscreen', 'codeview', 'help']]
        ],
    }).on("summernote.enter", function(we, e) {
        $(this).summernote('pasteHTML', '<br />&VeryThinSpace;');
        e.preventDefault();
    });
    $("#description").summernote("code", noXXS);
</script>

gabuchoripper · 2021-10-26T19:18:42Z

for some people looking this on php, i was using some regex to find scripts tags and codes inside and delete them...
would be like code below:

        $code = preg_replace('/<script>[\s\S]+<\/script>/gm' , '' , $_POST['code']);

DennisSuitters · 2021-10-27T03:53:38Z

A better option when outputting to the browser using PHP, is to use a proven library like HTMLPurifier, it's easy, and properly sanitizes the output, esp. for XSS vulnerabilities. However, as I stated above, we really need a solution that works inside Summernote to be able to cover every programming language scenario, and mainly as new coders often lack the knowledge to sanitize data properly.

DennisSuitters · 2022-04-11T00:32:38Z

It's been brought to my attention, that the dropzone is a potential XSS security risk, with the reporter indicating the following like could be the culprit:

summernote/src/js/module/Dropzone.js

Line 107 in a8ebe18

$(content).each((idx, item) => {

I don't think it's the code in the line per see, but rather that that function doesn't sanitize the data before calling the insertNode for the item.
I think we need to revisit this issue, and possibly create a working sanitation function that can be called to sanitize data before insertion.

bananabr · 2022-04-16T01:42:57Z

It's been brought to my attention, that the dropzone is a potential XSS security risk, with the reporter indicating the following like could be the culprit:

summernote/src/js/module/Dropzone.js

Line 107 in a8ebe18

$(content).each((idx, item) => {

I don't think it's the code in the line per see, but rather that that function doesn't sanitize the data before calling the insertNode for the item.
I think we need to revisit this issue, and possibly create a working sanitation function that can be called to sanitize data before insertion.

I am the reporter for this issue. $(content) is also vulnerable, as giving an external user control over the Jquery global function call parameter is an XSS vector. content would need to be sanitized before the $() function is called.

bananabr · 2022-04-16T01:50:00Z

summernote/src/js/module/Dropzone.js

Line 105 in a8ebe18

this.context.invoke('editor.pasteHTML', content);

is vulnerable for another reason. In this case, the user input eventually reaches

summernote/src/js/core/range.js

Line 573 in 7ab83a1

const contentsContainer = $('<div></div>').html(markup)[0];

and the non-sanitized markup argument is the culprit.

stevenluengo · 2024-04-08T21:11:11Z

Hello,

Just bumping this as it's still ongoing issue Snyk is detecting in our repositories:

https://security.snyk.io/vuln/SNYK-JS-SUMMERNOTE-5905997?_gl=1%2a1qjjmk6%2a_ga%2aODQ3MDQ0NjkuMTY4MzgxNjExMw..%2a_ga_X9SH3KP7B4%2aMTcxMjYwNjI5NS4zNDIuMS4xNzEyNjA2MzMzLjAuMC4w

DennisSuitters · 2024-04-09T00:15:28Z

That's ok, no one has come up with a workable solution so far. Been a while too.

Asjidkalam and others added 9 commits May 22, 2020 16:03

Fixed XSS - summernote-lite.js

e2c6245

Fixed XSS - summernote.js

baafdf4

Fixed XSS - summernote-bs4.js

8fb7050

Update README.md

5a184a2

Update README.md

4340a06

Merge pull request #7 from benharvie/develop

d3a2b61

Test

Update README.md

6117abc

Merge pull request #8 from benharvie/develop

71fda69

Test

Merge pull request #1 from Asjidkalam/develop

44d9f32

Fixed Cross Site Scripting (XSS) in editor

DennisSuitters requested review from easylogic, hackerwins and lqez June 16, 2020 02:15

R2116 mentioned this pull request Jun 24, 2020

Script tags are executed when switching out of codeview #1462

Closed

JamieSlome mentioned this pull request Jun 25, 2020

Please stop executing encoded tags... (XSS ISSUE) #3719

Closed

R2116 reviewed Jun 29, 2020

View reviewed changes

dist/summernote.js Outdated Show resolved Hide resolved

R2116 reviewed Jun 29, 2020

View reviewed changes

dist/summernote-lite.js Outdated Show resolved Hide resolved

R2116 reviewed Jun 29, 2020

View reviewed changes

dist/summernote-bs4.js Outdated Show resolved Hide resolved

JamieSlome and others added 3 commits June 30, 2020 11:34

Update dist/summernote.js

cdbe060

Co-authored-by: R2116 <15418932+R2116@users.noreply.github.com>

Update dist/summernote-lite.js

43b54fa

Co-authored-by: R2116 <15418932+R2116@users.noreply.github.com>

Update dist/summernote-bs4.js

dc00dd8

Co-authored-by: R2116 <15418932+R2116@users.noreply.github.com>

NVV-WebManager mentioned this pull request Nov 25, 2020

[FEATURE] Request to update Summernote plugin to latest version ColorlibHQ/AdminLTE#3149

Closed

DennisSuitters mentioned this pull request Nov 27, 2020

Two vulnerabilities #3913

Closed

do the changes in /src folder instead of /dist

81ea066

DennisSuitters mentioned this pull request Feb 6, 2021

Summernote executes escaped html #3985

Closed

DennisSuitters mentioned this pull request Jul 22, 2021

Trying to get in touch regarding a security issue #4098

Closed

DennisSuitters mentioned this pull request Aug 17, 2021

Input is HTML-decoded before render requiring unusual sanitization to be used #4125

Closed

dennisreimann mentioned this pull request Sep 8, 2021

Fix Summernote XSS possibility btcpayserver/btcpayserver#2859

Merged

lqez self-assigned this Sep 17, 2021

wrongecho mentioned this pull request Jan 4, 2022

Ticket Details is vulnerable to XSS itflow-org/itflow#232

Closed

DennisSuitters mentioned this pull request Feb 2, 2022

v0.8.20 Bug <script> Still Executed When Set Code #4218

Closed

philippkraft mentioned this pull request Feb 10, 2022

Paste and Copy of formatted text can cause strange behaviour freescout-helpdesk/freescout#1672

Closed

DennisSuitters mentioned this pull request Jun 10, 2022

Cross Site Scripting (XSS) on adding link or media #4306

Closed

netsevdam mentioned this pull request Jun 11, 2022

Summernote xss protected #4307

Closed

dribba added a commit to ubreakifix/summernote that referenced this pull request Jul 10, 2022

fix(xss): See summernote/summernote#3782

03f77bd

DennisSuitters mentioned this pull request Feb 2, 2023

Code View XSS #4423

Closed

JVT038 mentioned this pull request Jun 11, 2023

Add user movie reviews leepeuker/movary#388

Open

3 tasks

xet7 mentioned this pull request Jul 18, 2023

Calendar view logic wekan/wekan#4511

Closed

DennisSuitters mentioned this pull request Oct 11, 2023

Detected XSS with Snyk security tool #4533

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Security Fix for Cross Site Scripting - huntr.dev #3782

Security Fix for Cross Site Scripting - huntr.dev #3782

huntr-helper commented Jun 15, 2020 •

edited

R2116 commented Jun 29, 2020

JamieSlome commented Jun 30, 2020

Augustukas commented Oct 19, 2020

JamieSlome commented Oct 26, 2020

DennisSuitters commented Nov 26, 2020 •

edited

DennisSuitters commented Nov 30, 2020

DennisSuitters commented Nov 30, 2020

DennisSuitters commented Jan 6, 2021

DennisSuitters commented Jan 6, 2021

nuclearghost commented Jan 28, 2021

ivanovych666 commented Jan 29, 2021 •

edited

nuclearghost commented Jan 29, 2021

MrGoatsy commented Feb 6, 2021 •

edited

DennisSuitters commented Feb 6, 2021

MrGoatsy commented Feb 6, 2021

gabuchoripper commented Oct 26, 2021

DennisSuitters commented Oct 27, 2021

DennisSuitters commented Apr 11, 2022

bananabr commented Apr 16, 2022

bananabr commented Apr 16, 2022

stevenluengo commented Apr 8, 2024

DennisSuitters commented Apr 9, 2024

Security Fix for Cross Site Scripting - huntr.dev #3782

Are you sure you want to change the base?

Security Fix for Cross Site Scripting - huntr.dev #3782

Conversation

huntr-helper commented Jun 15, 2020 • edited

User Comments:

📊 Metadata *

Bounty URL: https://www.huntr.dev/bounties/1-npm-summernote

⚙️ Abstract

❓ Technical description

🐛 Proof of Vulnerability (PoV)

🔥 Proof of Fix (PoF)

R2116 commented Jun 29, 2020

JamieSlome commented Jun 30, 2020

Augustukas commented Oct 19, 2020

JamieSlome commented Oct 26, 2020

DennisSuitters commented Nov 26, 2020 • edited

DennisSuitters commented Nov 30, 2020

DennisSuitters commented Nov 30, 2020

DennisSuitters commented Jan 6, 2021

DennisSuitters commented Jan 6, 2021

nuclearghost commented Jan 28, 2021

ivanovych666 commented Jan 29, 2021 • edited

nuclearghost commented Jan 29, 2021

MrGoatsy commented Feb 6, 2021 • edited

DennisSuitters commented Feb 6, 2021

MrGoatsy commented Feb 6, 2021

gabuchoripper commented Oct 26, 2021

DennisSuitters commented Oct 27, 2021

DennisSuitters commented Apr 11, 2022

bananabr commented Apr 16, 2022

bananabr commented Apr 16, 2022

stevenluengo commented Apr 8, 2024

DennisSuitters commented Apr 9, 2024

huntr-helper commented Jun 15, 2020 •

edited

DennisSuitters commented Nov 26, 2020 •

edited

ivanovych666 commented Jan 29, 2021 •

edited

MrGoatsy commented Feb 6, 2021 •

edited