search

brianvoe / slim-select

Slim advanced select dropdown
http://slimselectjs.com
MIT License
1.05k stars 200 forks source link

Cross Site Scripting #564

Closed Skileau closed 2 months ago

Skileau commented 2 months ago

Describe the bug I have found a Cross Site Scripting in the library, where should I submit the PoC?

brianvoe commented 2 months ago

here

Skileau commented 2 months ago

Because of the use of innerHTML here instead of innerText, it is possible to inject HTML which is interpreted: https://github.com/brianvoe/slim-select/blob/master/src/slim-select/select.ts#L345

I used the following HTML page as an example:

<html>
  <head>
    <script src="https://unpkg.com/slim-select@latest/dist/slimselect.min.js"></script>
    <link href="https://unpkg.com/slim-select@latest/dist/slimselect.css" rel="stylesheet"></link>
  </head>
  <body>
    <select id="selectElement">
      <option>&lt;IMG SRC=X ONERROR=alert(2)&gt;&lt;/IMG&gt;</option>
      <option>&#x3c;&#x69;&#x6d;&#x67;&#x20;&#x73;&#x72;&#x63;&#x3d;&#x78;&#x20;&#x6f;&#x6e;&#x65;&#x72;&#x72;&#x6f;&#x72;&#x3d;&#x61;&#x6c;&#x65;&#x72;&#x74;&#x28;&#x31;&#x29;&#x3e;&#x3c;&#x2f;&#x69;&#x6d;&#x67;&#x3e;</option>
      <option>Option 3</option>
    </select>
    <script>
      new SlimSelect({
        select: '#selectElement'
      })
    </script>
  </body>
</html>

It is important to note that the payload is HTML encoded.

Changing the selected option on the page leads to the execution of JavaScript: image

brianvoe commented 2 months ago

Sorry i just dont see this as a slim select issue.

brianvoe commented 1 month ago

@Skileau You told another company about this?

Skileau commented 1 month ago

Yes, I reported it during a pentest as it's my job and the usual process, and reached out to you so we can secure this issue upstream.

brianvoe commented 1 month ago

Your letting someone submit XSS attacks through your input fields, save them into the database, serve them up to another user, inject it into a ui select dropdown and want to say the ui select dropdown is the issue?

brianvoe commented 1 month ago

If you have a fix for this that doesn't effect how things currently work. Submit a pr

Skileau commented 1 month ago

Is there a specific reason why you are using innerHTML rather than innerText here ? https://github.com/brianvoe/slim-select/blob/master/src/slim-select/select.ts#L377C14-L377C23

brianvoe commented 1 month ago

Nope, dont care what value it is as long as it works. If you have a fix for this that doesn't effect how things currently work. Submit a pr

Skileau commented 1 month ago

I am not a developer so I would not be able to provide you with a fix and confirm that it would not break anything, sorry. I can simply confirm you that the part of the code that I exploited to get an XSS is this specific innerHTML: https://github.com/brianvoe/slim-select/blob/master/src/slim-select/select.ts#L377C14-L377C23

You can try to replace it with an innerText and build the application on your side to make sure that it works well. As previously said, here is the index.html that I used for a small PoC:

<html>
  <head>
    <script src="https://unpkg.com/slim-select@latest/dist/slimselect.min.js"></script>
    <link href="https://unpkg.com/slim-select@latest/dist/slimselect.css" rel="stylesheet"></link>
  </head>
  <body>
    <select id="selectElement">
      <option>&lt;IMG SRC=X ONERROR=alert(1)&gt;&lt;/IMG&gt;</option>
      <option>Option 2</option>
      <option>Option 3</option>
    </select>
    <script>
new SlimSelect({
  select: '#selectElement',
})
    </script>
  </body>
</html>

As long as once you change the selection and it does not trigger the alert(1), then the reported vulnerability can be considered as fixed.

Also notice that this example is provided as a PoC but I exploited this vulnerability in a global application during a security assessment to elevate my privileges to super admin as I was able to inject a malicious payload into one of the list. Even though it was their choice to let users create new options, I think that this is a frequent use case and it cannot be ignored ... Especially as there doesn't seem to be any real point in converting the option's innerText into HTML, so the fix is unlikely to break any feature.

brianvoe commented 1 month ago

"I am not a developer" - ok thanks have a good one

Zerotask commented 3 weeks ago

Dependabot also warned us about a potential risk for slimselect: https://nvd.nist.gov/vuln/detail/CVE-2024-9440

Slim Select 2.0 versions through 2.9.0 are affected by a potential cross-site scripting vulnerability. In select.ts:createOption(), the text variable from the user-provided Options object is assigned to an innerHTML without sanitation. Software that depends on this library to dynamically generate lists using unsanitized user-provided input may be vulnerable to cross-site scripting, resulting in attacker executed JavaScript. At this time, no patch is available.

Shoplifter commented 3 weeks ago

I can't see any reason why innerText should not be used here instead of innerHTML. In contrast: In an option tag only text is permitted anyways see: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/option#technical_summary

brianvoe commented 3 weeks ago

update the code and add a test. submit a pr and ill get it in.

Shoplifter commented 3 weeks ago

Hi @brianvoe I just submitted a PR for that. ~~Don't know what you mean with "add a test" I think the update should be already covered by all the tests that use the select.createOption method~~

UPDATE: the original PR did not pass the test suite. I changed innerText to textContent. Also added a test for this and now all tests pass.

brianvoe commented 3 weeks ago

v2.9.2

frenkel commented 2 weeks ago

Thanks for fixing this @Shoplifter!