miroslavpejic85 / mirotalk

🚀 WebRTC - P2P - Simple, Secure, Fast Real-Time Video Conferences Up to 8k and 60fps, compatible with all browsers and platforms.
https://p2p.mirotalk.com
GNU Affero General Public License v3.0
3.12k stars 570 forks source link

Multiple Cross-Site Scripting (XSS) #139

Closed l4rm4nd closed 1 year ago

l4rm4nd commented 1 year ago

Describe the bug

The mirotalk web application is susceptible to XSS.

If two participants joined a web meeting, one participant can change their name, which is then reflected for all other meeting participants. Since no input validation takes place; or output filtering to sanitize maliciously choosen names, the payload of an attacker is executed in the browser of other victims.

This can introduce various problems and attack vectors, such as:

To Reproduce

Steps to reproduce the behavior:

  1. Go to https://p2p.mirotalk.com/ and create a new room. Join the room yourself.
  2. Start a second private browser window to simulate another user. Join the same room via invite link e.g.
  3. After joining the meeting, change your name via settings to <img src="x" onerror=alert(document.domain)>
  4. Verify a JavaScript popup in both browser meeting rooms. You successfully XSS'd other users.

Expected behavior

A meeting participant's name should always be sanitized before being displayed or embedded into the website's content.

Screenshots

image

Desktop - Mobile

Please complete the following information:

Additional context

Recommendation

Other from that, many thanks for this projects. Really, really cool!

l4rm4nd commented 1 year ago

BTW, the file upload function (share a file) is also susceptible.

Any user can upload a file with a XSS payload in the filename. For example Sun'><img src=x onerror=alert(1)>set.jpg.

As the filename is reflected at multiple locations in the web application (e.g. in the chat screen or upload popup), this again can XSS other meeting participants.

image

image

miroslavpejic85 commented 1 year ago

Hello @l4rm4nd,

Thanks so much for reporting this to me, it should be fixed now. XSS no longer runs in the participants room, it is blocked before it is sent.

Other from that, many thanks for this projects. Really, really cool!

Thank you so much, I'm very happy you like it!

Join with us on Discord and Have a good weekend.

l4rm4nd commented 1 year ago

Hi @miroslavpejic85,

many thanks for the fast reply and fix.

Your fix works as intended on the client side. Malicious names or filenames are not transfered to other participants but blocked before sending.

However, this validation only happens on the client side via JS. An attacker can easily intercept a valid name modification or file transfer via websockets (that is not blocked before sending) and just modify the request directly. On the participant's side, no validation or sanitization will take place and the XSS payloads will trigger again.

An example websocket request for file transfer would look like this:

42["fileInfo",{"room_id":"08662TinyTowel","broadcast":true,"peer_name":"Kali","peer_id":"am6Y_BhRyop_UOzkAAmj","file":{"fileName":"<img src=x onerror=alert(document.domain)>","fileSize":156,"fileType":"image/png"}}]

image

So your fix does not mitigate XSS, unfortunately.

Here an example for name changing via websockets:

image

l4rm4nd commented 1 year ago

It's a general problem with output sanitization, since input validation will always be easily bypassed due to JS and websockets.

For example, the raising hand feature is also susceptible. An attacker can easily modify the websocket request and again modify the peer_name. XSS on all participants, as the peer_name is reflected.

42["peerStatus",{"room_id":"97337HotCouch","peer_name":"<img src=x onerror=alert(document.domain)>","element":"hand","status":true}]

image

l4rm4nd commented 1 year ago

Same for sharing a video. Peer name can again be weaponized for XSS. The opened video player on the participant's side will reflect which peer opened the video.

42["videoPlayer",{"room_id":"97337HotCouch","peer_name":"Kali<img src=x onerror=alert(document.domain)>","video_action":"open","video_src":"https://www.youtube.com/embed/uxELAbwB19c?autoplay=1","peer_id":null}]

image

BTW, this also allows for impersonification. So an attacker could easily fake the peer_name and specify another participants name and share an inappropriate video. Other participants will only see the text " opened a video..." and would be like "what the f*ck ....".

image

l4rm4nd commented 1 year ago

So sorry to spam the project with security issues but I really love this project.

It is so much better than other web meeting platforms and provides a lot of cool and thoughtful features.

If we get things like these sorted, it's just brilliant <3!

miroslavpejic85 commented 1 year ago

Hi @l4rm4nd, Don't worry, I'm very glad you care about security!

I added the XSS sanitization on the server side, in theory now should everything be fixed regarding the xss?

Can you confirm please? Thank you!

It is so much better than other web meeting platforms and provides a lot of cool and thoughtful features.

Thank you for your nice feedback! :)

If we get things like these sorted, it's just brilliant <3!

Sure, Thanks to your cooperation everything will be improved - fixed ;)

l4rm4nd commented 1 year ago

hi @miroslavpejic85,

good job using the js-xss library. Looks like the XSS is successfully fixed.

I've even tried some older XSS payloads to especially bypass js-xss from Portswigger XSS Cheatsheet. No luck.

<script>Object.prototype.whiteList = {img: ['onerror', 'src']}</script><script>document.write(filterXSS('<img src onerror=alert(1)>'))</script>

The only thing that is now still possible is HTML injection. But honestly, the impact for this is kinda low.

image

image

And some self-XSS popups that are not exploitable for other participants. Will only trigger a JS popup for the attacker himself. Should be fixed too but there is no security impact/risk leaving it as is.

For example a name change to <img src=x onerror=alert(1)> or a file upload with filename of Sun'><img src=x onerror=alert(1)>set.jpg gets blocked by your client.js fix and the messages invalid name and filename are shown. However, the JS payload is nonetheless executed (only in the attackers browser though; so a self-xss issue with no impact).

image

miroslavpejic85 commented 1 year ago

Hi @l4rm4nd,

Nice to hear your again.

good job using the js-xss library. Looks like the XSS is successfully fixed.

Thank you, yes, seems very good!

The only thing that is now still possible is HTML injection. But honestly, the impact for this is kinda low.

I agree with you.

And some self-XSS popups that are not exploitable for other participants. Will only trigger a JS popup for the attacker himself. Should be fixed too but there is no security impact/risk leaving it as is.

For the name change, I found a way to disable the JS Popup and clean up :) For the file name, seems I can't do it, as it's read only, but is xss with no impact.

Thanks again so much for reporting this issue to me and for helping with debugging, much appreciated!

l4rm4nd commented 1 year ago

Hi @miroslavpejic85 ,

very nice. The self XSS for the name change function is now fixed.

I've found a remaining self XSS when joining a meeting. You may want to fix this too as it is the second input field presented to a user (after optionally choosing a room name).

image

image

You may want to restrict and filter the room name people can choose. A room name with </h1> or just </> bricks the loading screen.

image

l4rm4nd commented 1 year ago

Hi @miroslavpejic85 ,

sorry, I've found another XSS. This time it is a bit more hidden.

A user can choose an XSS payload as name when joining a web meeting. The XSS name will trigger a self XSS popup for the attacker himself. No impact for other participants.

However, the private messages feature allows to search for meeting participants and sending them a private message. When a message is sent, the chat will print a text like Private message to <peer_name>. Malicious peer name allows for XSS again.

Proof of Concept:

  1. Attacker chooses XSS as name during meeting join
  2. Another participant sends the attacker with XSS payload in his name a private message
  3. The victim who sent a message to the attacker triggers the JS payload

imageImage1: Attacker sets his name to an xss payload

imageImage2: Victim want to send a private message to the attacker

imageImage3: Attacker's peer name is reflected in the chat which triggers XSS

So the reflection of peer_name for private messages must be validated and sanitized too.

miroslavpejic85 commented 1 year ago

Hi @l4rm4nd, Yeah you are right, can you try now please? should be fixed. Thank you!

l4rm4nd commented 1 year ago

Hi @l4rm4nd, Yeah you are right, can you try now please? should be fixed. Thank you!

Looks good! Malicious HTML/JS is escaped properly.

image

Participants can also not choose malicious payloads anymore during meeting join:

image

Also within meeting a peer name cannot be changed to malicious JS/HTML on the frontend:

image

Great job and many thanks for your fast replies and implemented fixes!

🥇 👍

miroslavpejic85 commented 1 year ago

Good! It's a pleasure, Many thanks to you!