moode-player / moode

moOde sources and configs
GNU General Public License v3.0
984 stars 163 forks source link

XSS found in metadata #680

Open n3bojs4 opened 1 week ago

n3bojs4 commented 1 week ago

Hello,

I found a XSS vulnerability in the media player. You can inject javascript code into media files metadatas. To reproduce, open mp3 file with vlc for example, change the Artist field to "inXSS" for example. Open the media player and play the file.

Capture d’écran du 2024-09-05 23-32-54

Version : Release 9.0.8 2024-08-21

Regards.

moodeaudio commented 1 week ago

I'm not a security expert but I'll be happy to implement mitigation if you can explain what approach to take.

n3bojs4 commented 1 week ago

The problem stems from the fact that the metadata contained in the files is interpreted by the browser. I think the code in question is in the playbar. In fact, the playbar-currentsong element should display text and not html code. To avoid this kind of problem, there are several points to address.

  1. try to manage user input. In this case, it's complicated because it's a file, not a form filled in through the browser.
  2. Encode the output data, using html entity encoding.
  3. Set up a CSP, or Content Security Policy.

This is 2 useful resources to help you in understanding what is a XSS and how to prevent it !

https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html

https://portswigger.net/web-security/cross-site-scripting

PS: you'll need to take into account all the metadata fields you want to display in the player!

moodeaudio commented 1 week ago

I read those articles and it looks like the vulnerability you are demonstrating falls into the "Stored XSS" scenario. To mitigate this I suppose the file metadata elements could be checked for unwanted characters or commands. If they are present then replace them with "?" which prevents the code from being executed by the Browser, then log a security error and the data that failed the test.

Unwanted characters would include "<", ">", "=", "/" Unwanted commands would include "script", "http"

n3bojs4 commented 1 week ago

Yes, we can talk about stored xss because it's persistent. Your approach seems correct, but watch out for the encoding. In my experience, it's very difficult, if not almost impossible, to perform XSS when the developer makes sure that the display is encoded with html entities. In the OWASP article, I take the liberty of focusing on this part:

If you're using JavaScript for writing to HTML, look at the .textContent attribute. It is a Safe Sink and will automatically HTML Entity Encode.

&    &amp;
<    &lt;
>    &gt;
"    &quot;
'    &#x27;
moodeaudio commented 1 week ago

That could prolly work but it doesn't get rid of the malicious code thats stored in the file. I'm guessing it's not common to find malicious code in song files, at least no one on our Forum has ever reported something like this.

The metadata could be filtered on the back-end but that imposes a performance penalty for the vast majority of collections that have no bad files, and it doesn't actually get rid of the bad code.

The same type of filter could be applied to metadata in the front-end but there are the same issues of extra processing even though most collections have clean metadata, and this approach also doesn't actually get rid of the bad code.

Another option is to create a utility that scans files and reports occurrences of malicious code embedded in metadata. The user can then go in and clean it up or delete the file.

MrSeccubus commented 1 week ago

@moodeaudio it is typically not the correct approach to try an filter these elements in ingest, especially in you case where you have no control over the files a user is going to load into you media player. Instead it is better to make sure that these characters are converted to their relevant HTML entiries before they are returned to the browser for rendering.

In PHP youhave the funciton htmlspecialchars() which takes care of this converstion for you. In Javascript a function to turn text into it's text representation could be something like:

function encodeInput (input)  {
  const encoded = document.createElement('div');
  encoded.innerText = input;
  return encoded.innerHTML;
}
moodeaudio commented 1 week ago

Yes filtering/converting in the front-end would be one approach but the problem is that it leaves the user with no clue that there is malicious code in a music file unless the filter does detect/filter/log/alert rather than just converting unsafe characters to safe characters.

We recently worked with a security firm to implement mitigation for code and SQL injection in our PHP templates and command API's and this was done with back-end detect/filter/log routines. The perf penalty was insignificant since the number of variables being checked during a typical transaction was on average prolly ten.

Doing back-end detection for code in metadata tags would be preferable but since Moode's Library Gen is a batch process with caching it presents a perf issue since the number of files tags to be checked can be very large for example 25K files 10 tags/file = 250K tags.

Moode also provides an on-demand query of tags in the MPD database and so that back-end code would also need the detection/filtering routine.

I did do various Google searches for malicious code in music metadata tags but the results I got referenced steganography in wav files and not XSS. If you have any links specific to music files please post.

My assessment at this point is:

  1. The severity of this particular vulnerability is not very high.
  2. Implementation should be on the back-end with detect/filter/log so user is informed.
  3. I've added it to the TODO list and created a chkXSS() function but TBD on implementation due to limited dev bandwidth.
n3bojs4 commented 1 week ago

I did do various Google searches for malicious code in music metadata tags but the results I got referenced steganography in wav files and not XSS. If you have any links specific to music files please post.

Using metadata in files to perform injection attacks, particularly XSS, is not uncommon. Just google “xss metadata” and you're busy for the rest of the day. Even big projects like Wordpress and Nextcloud, to name but a few, have had to deal with this kind of problem.

The severity of this particular vulnerability is not very high.

For your information, using the CVSS V3 scoring system, this vulnerability could be categorized as medium.

https://nvd.nist.gov/vuln-metrics/cvss/v3-calculator?vector=AV:N/AC:L/PR:N/UI:R/S:U/C:L/I:L/A:L

I think I've done all I can to help and inform you about this problem. It's good to know that you're planning to take it into account. Bon courage !

moodeaudio commented 1 week ago

Ok, thanks for all the help. I'll close for now and then reopen if there are any updates from my end.

MrSeccubus commented 1 week ago

To be honest I think that's a bit unfair from your side. You users deserve to know that this issue exists.

MrSeccubus commented 1 week ago

Also I think you remark about notifying the user is commendable, but first of all you users deserve not to get owned above a warning that somebody tried to own them.

If you want to implement detection and warning think of it as an additional feature and fixing the XSS as a bug fix.

moodeaudio commented 1 week ago

The vast majority of our users are not devs that have GitHub accounts and so they will never see this issue.

If you want to make our user community aware then visit the Support page at moodeaudio.org and request a Forum account. Be sure to include your preferred userid (at least 5 characters). Then go ahead and post a Thread in the Support sub-Forum and report the vulnerability including a link to this Git issue. If myself or another dev makes progress on the issue the Thread will get updated.

Btw, you did not provide any links which I requested that specifically reference security articles discussing XSS code embedded in music file metadata. If you have such links include them in your Forum Thread.

moodeaudio commented 6 days ago

I've implemented a back-end mitigation that consists of filtering metadata using PHP htmlspecialchars() plus an XSS detect/log feature that provides the user with a list of files/tags containing XSS characters or commands.

Link to Forum post https://moodeaudio.org/forum/showthread.php?tid=6887&pid=57549#pid57549

n3bojs4 commented 6 days ago

I've implemented a back-end mitigation that consists of filtering metadata using PHP htmlspecialchars() plus an XSS detect/log feature that provides the user with a list of files/tags containing XSS characters or commands.

Link to Forum post https://moodeaudio.org/forum/showthread.php?tid=6887&pid=57549#pid57549

It's a good news Tim but render unto Caesar the things that are Caesar's

The XSS issue was raised by Git user @MrSeccubus. Link below. https://github.com/moode-player/moode/issues/680

moodeaudio commented 6 days ago

lol, I'll make the correction.