Possible XSS attack

[NomNuggetNom]

Requirements

• This is a bug report, and if not, please post to https://lemmy.ml/c/lemmy_support instead.
• Please check to see if this issue already exists.
• It's a single bug. Do not report multiple bugs in one issue.
• It's a frontend issue, not a backend issue; Otherwise please create an issue on the backend repo instead.

Summary

The sidebar dangerously sets HTML but does not configure the Markdown render to strip HTML codes. This enables simple XSS attacks like <img onload="maliciousCodeHere()" />. It seems like an attempt is made to create a markdown renderer with HTML disabled, however.

It now seems that this attack might be done via custom emojis.

Steps to Reproduce

Technical Details

markdown-it does some extremely simple guarding, but they don't claim to prevent XSS. Custom HTML should be removed in favor of plugins.

Lemmy Instance Version

0.18.1

Lemmy Instance URL

No response

[geneccx]

The issue is not with the sidebar, but the markdown renderer.
Anywhere that markdown can be used is a vector.

I've emailed details to the devs, but @makotech222 has already identified a fix.

[tgxn]

Also, this needs to be fixed on the backend, client-side validation is never enough. @geneccx - Where can I find the fix that you mention @makotech222 contributed on?

[geneccx]

Also, this needs to be fixed on the backend, client-side validation is never enough. @geneccx - Where can I find the fix that you mention @makotech222 contributed on?

#1897

[calculuschild]

@NomNuggetNom please keep the issue open until it has been fixed. We want to be sure the Devs see this.

[geneccx]

After implementing this fix, you will likely also need to invalidate all existing sessions (logging everyone out) due to LemmyNet/lemmy#3499

In the meantime,

From the matrix chat:

Current mitigations

1. Remove custom emoji

DELETE FROM custom_emoji_keyword;
DELETE FROM custom_emoji;

2. Overwrite content with the exploit

UPDATE comment SET content = '<REMOVED BY ADMIN>' WHERE content LIKE '%![" onload%';
UPDATE private_message SET content = '<REMOVED BY ADMIN>' WHERE content LIKE '%![" onload%';
UPDATE post SET body = '<REMOVED BY ADMIN>' WHERE body LIKE '%![" onload%';
UPDATE post SET name = '<REMOVED BY ADMIN>' WHERE name LIKE '%![" onload%';

3. Rotate your JWT secret (invalidates all current login sessions)

-- back up your secret first, just in case
SELECT * FROM secret;
-- generate a new secret
UPDATE secret SET jwt_secret = gen_random_uuid();

4. Restart lemmy to make use of the new secret.

[AlmightySnoo]

Both the sidebar and the legal information field don't seem to escape strings? The "Legal" page in lemmy.world is allowing malicious JavaScript to run right now.

[geneccx]

Both the sidebar and the legal information field don't seem to escape strings? The "Legal" page in lemmy.world is allowing malicious JavaScript to run right now.

It's custom emoji rendered anywhere. The mitigation is above, deleting your custom emojis until the fix is in place, and invalidate all sessions.

[AlmightySnoo]

Both the sidebar and the legal information field don't seem to escape strings? The "Legal" page in lemmy.world is allowing malicious JavaScript to run right now.

It's custom emoji rendered anywhere.

EDIT: I'm wrong, it's indeed the emoji.

[arifwn]

Rotate your JWT secret (invalidates all current login sessions)

Looks like you'll need to restart lemmy to make it uses the new secret.

[geneccx]

Rotate your JWT secret (invalidates all current login sessions)

Looks like you'll need to restart lemmy to make it uses the new secret.

Thanks, I'll add that to the list of steps.

[geneccx]

@tgxn teaching more people how to exploit it in the wild probably isn't the best idea...

[tgxn]

@tgxn teaching people how to exploit it in the wild probably isn't the best idea...

Sure, I'll delete the post; but it's not like malicious actors care, we know that it's specifically:

1. Custom Emoijs are being exploited.
2. Site admins removing custom emojis should remove this attack vector.

[abhibeckert]

@tgxn teaching people how to exploit it in the wild probably isn't the best idea...

It was already being executed in the wild on some of the largest instances before this issue was created.

[geneccx]

It was already being executed in the wild on some of the largest instances before this issue was created.

I am aware. The fix and mitigations came after discovering it being used in the wild. The point still stands, until more servers have had the chance to patch this up, and the Lemmy devs have had a chance to release the fix, sharing the specifics of how to execute the exploit can only cause more harm than good.

[StephenWetzel]

UPDATE comment SET content = '<REMOVED BY ADMIN>' WHERE content LIKE '%![" onload%';
UPDATE private_message SET content = '<REMOVED BY ADMIN>' WHERE content LIKE '%![" onload%';
UPDATE post SET body = '<REMOVED BY ADMIN>' WHERE body LIKE '%![" onload%';
UPDATE post SET name = '<REMOVED BY ADMIN>' WHERE name LIKE '%![" onload%';

Should these be ILIKE so the attackers can't just change the case and escape detection?

[stirante]

Maybe quick and easy way to fix it would be to either use https://www.npmjs.com/package/safe-marked or adapt current solution to use https://www.npmjs.com/package/dompurify
EDIT: Also found this plugin for existing markdown-it library: https://www.npmjs.com/package/markdown-it-dompurify

As always no user input can be trusted.

[Nutomic]

What makes you think that custom emojis are used as the attack vector? The fact is that emojis can only be set by local admins, and they dont federate. So I fail to see how an attack could set emojis, unless he already gained admin privileges through another attack vector.

[dcx]

What makes you think that custom emojis are used as the attack vector? The fact is that emojis can only be set by local admins, and they dont federate. So I fail to see how an attack could set emojis, unless he already gained admin privileges through another attack vector.

I'm doing some triage right now. I'm not sure if I fully understand this yet, but this is in my instance's database. I think this means it will be injected if our users click into the infected discussion?

(Edit: Scrubbed to minimise proliferation)

[abhibeckert]

I think this means it will be injected if our users click into the infected discussion?

Anyone has viewed a page where that onload code executes has had their login token sent to a third party server. Allowing whoever runs that server to impersonate them.

If you haven't already, you should take your instance offline until this has been sorted out.

[sunaurus]

Those comments from the screenshot are all from lemmy.world, so they would not have any effect on the poster's instance. Or @abhibeckert are you aware of an additional vulnerability other than what has been discussed here?

[abhibeckert]

Those comments from the screenshot are all from lemmy.world, so they would not have any effect on the poster's instance. Or @abhibeckert are you aware of an additional vulnerability other than what has been discussed here?

No if I was aware of anything you guys don't know I would've offered it.

I was just answering their question, since they seemed to be unclear what the code does (and what the risk is).

[SheriffJake]

I'm doing some triage right now. I'm not sure if I fully understand this yet, but this is in my instance's database. I think this means it will be injected if our users click into the infected discussion?

That code resolves to a url: https://zelensky[.zip]/save/navAdmin
Virustotal has so far only one detection with it, but that might change soon:
https://www.virustotal.com/gui/url/c2bb82f2bd01786c16f4425544e9d5d080fe3302fa139eb67ad1744890f95e4f?nocache=1

[dcx]

As a precaution, I've run the measures suggested by @geneccx above (thanks!) and disabled federation for now. I've provided the full list of ten below for convenience. Removed to minimise proliferation, DM me if you're a lemmy dev and would like the code.

Hmm. This is not important for me personally now, but I still don't yet understand how this doesn't affect users on my local instance, who view the "infected" comment (which came from the lemmy.world federated post, but was living in my db). It seems to run on every browser onLoad and send the user's cookie.

I'm not a frontend guy. I assume by the developers' comments that these must be escaped in some way.

[phiresky]

Mentioning #1900 which should prevent this and most other XSS attacks without modifying the db or other code (barely tested)

[DarkIrata]

**AlmightySnoo ** commented Jul 10, 2023 •

i know, thats why i said workaround. It would be more than just waiting for an update and gettting users cookies snatched in the time. If everyone disbles federation for the time, the whole point of lemmy is more or less lost for the time.

[geneccx]

that means that those custom emojis in the federated comments are likely to be the XSS vectors. It remains to see where exactly in lemmy-ui those comments aren't escaped properly.

No, malicious comments (and other activities) need to be specific to the instance's custom emoji for the exploit to work.

From my testing and observations, federated activities referencing custom emoji from those other instances do not trigger the issue.

[FireMasterK]

The current fix in #1897 doesn't really do anything when the emoji itself has a malicious image URL or alt text. This was also pointed out by #1897 (comment)

My fix sanitizes everything in #1906, just to be safe.

[kevincox]

I agree that the code can still be improved. I filed #1904 to track proper HTML generation. Even your solution seems a bit strange as it generates possibly invalid or malicious HTML then tries to clean it, I think it would be better to just avoid generating invalid or malicious HTML in the first place.

[AlmightySnoo]

2. Overwrite content with the exploit

UPDATE comment SET content = '<REMOVED BY ADMIN>' WHERE content LIKE '%![" onload%';
UPDATE private_message SET content = '<REMOVED BY ADMIN>' WHERE content LIKE '%![" onload%';
UPDATE post SET body = '<REMOVED BY ADMIN>' WHERE body LIKE '%![" onload%';
UPDATE post SET name = '<REMOVED BY ADMIN>' WHERE name LIKE '%![" onload%';

Just to be cautious, I'd also do the same with onerror with something like '%![%](" onerror%' as the following is also a possible XSS:

<img src="" onerror="something()">

as onerror will be triggered because the src attribute is empty.

I'd also do the same without a space between onload/onerror and the first quotation mark.

[kevincox]

This is still incomplete. You could add two spaces instead of one and bypass that pattern. It is also one-time only. The only true fix is to push out an updated UI.

[geneccx]

Yep, there's no way to identify all possible cases where this is done.

Update the frontend - the fix is available in 0.18.2-rc.1.

Deleting the comments/posts is not technically necessary, it's more for cleanup afterwards anyway.

[FireMasterK]

Just to be cautious, I'd also do the same with onerror with something like '%![%](" onerror%' as the following is also a possible XSS:

<img src="" onerror="something()">

as onerror will be triggered because the src attribute is empty.

I'd also do the same without a space between onload/onerror and the first quotation mark.

That wouldn't be sufficient in my opinion, you could potentially inject a <script> tag altogether. There are so many ways to perform an XSS attack with the current attack vector.

[AlmightySnoo]

Just to be cautious, I'd also do the same with onerror with something like '%![%](" onerror%' as the following is also a possible XSS:
<img src="" onerror="something()">
as onerror will be triggered because the src attribute is empty.
I'd also do the same without a space between onload/onerror and the first quotation mark.

That wouldn't be sufficient in my opinion, you could potentially inject a <script> tag altogether. There are so many ways to perform an XSS attack with the current attack vector.

Which means, as long as there isn't a proper fix to this, we still don't know whether instances currently host other malicious JavaScript code that is still active and discretely stealing cookies. We only know that we got rid of the onload's.

[sunaurus]

Just so we're all on the same page: the current fix wasn't getting rid of onloads, it was actually disabling the custom rendering logic for emojis.

For now, the only known way to actually run this code was through custom emojis (before 0.18.2-rc.1). We can speculate that there are other XSS vulnerabilities, but there are no known ones at the moment.

[AlmightySnoo]

I just checked again in the forum.basedcount.com instance which is running the 0.18.1 backend, and Markdown in the comments isn't properly escaped.

It is very curious however. The exploit requires a title that is exactly the name of any custom emoji (not necessarily a compromised one, it just needs to be valid!) in the Markdown image tag to work and the image URL can be anything, but without a title it doesn't work. For instance, putting this in a comment successfully injects the JavaScript:

![" onload="alert('pwned')"](any_image_url "name of a custom emoji, like tradwife or emily in the instance above") 

but this won't work:

![" onload="alert('pwned')"](any_image_url) 

Thus, a simple user without any admin rights can perfectly perform the exploit and attempt to steal admin cookies, so it's wrong to assume that it's exclusively a compromised custom emoji that is allowing the hack.

[geneccx]

@AlmightySnoo #1895 (comment)

It's usage of a local custom emoji by any user that triggers the exploit, and the PR fixes the usage piece such that normal users cannot inject arbitrary JS by using local custom emoji. Admins however, can still place arbitrary JS when creating an emoji.

The existing PR fixes the current issue that is being exploited in the wild.

[calculuschild]

I think their point is it's wrong to assume the icon is the only vulnerability, just because that's the approach the hackers used this time.

[geneccx]

Sure, but who's assuming that?

There's movement now in other areas, including adding a CSP, potentially revisiting the JWT implementation, among others.

There are going to be other exploits, that is inevitable. Focus is better directed towards layers of defence rather than speculation.

[abhibeckert]

@sunaurus I think it would be good to learn from this and add some automated method to quickly deploy urgent security patches. There are thousands of lemmy instances and it will take forever for all of them to update.

I'm not suggesting fully automated software updates, just some way to quickly patch a critical vulnerability like this one.

[rcmaehl]

@sunaurus I think it would be good to learn from this and add some automated method to quickly deploy urgent security patches. There are thousands of lemmy instances and it will take forever for all of them to update.

I'm not suggesting fully automated software updates, just some way to quickly patch a critical vulnerability like this one.

Maybe with an opt-in/opt-out option because I'm sure some very specific admins will be up in arms about the potential abuse.

[ornato-t]

Hey everyone, admin of forum.basedcount.com here. Sorry in advance for the trouble that this is giving to you. I'll go ahead and update my instance to the latest rc, as well as follow all the tips in this issue.

EDIT: in the meantime I've shut down the instance. I'm trying my hardest to stop this from proliferaring, at least through our instance.

[finnim]

Maybe with an opt-in/opt-out option because I'm sure some very specific admins will be up in arms about the potential abuse.

I would definitely welcome an API endpoint controlled by a few trusted instance admins that could be polled every minute or so with a very simple GET request.

In the case of a security issue that requires immediate attention by an admin or sysadmin I would have my server shutdown or stop all containers if it receives a certain agreed-upon response.

This is the only way we can have small instances run by few people who might be asleep without them becoming attack vectors.

Worst case it's a false positive and my instance goes down, but I rather have this than compromise any user data.

Thinking about it, this could also be a useful warning service for a multitude of scenarios. I'll consider setting up a button for authenticated users to press, in this case not to shut down the instance but to give me some sort of alert for medium-tier incidents.

[SexyVetra]

an API endpoint controlled by a few trusted instance admins

I think you meant to propose checking for GitHub for new releases instead of adding a new points of failure and additional vulnerability surfaces

[grahhnt]

For future reference, this was patched by commit e80bcf5 and became available in version v0.18.2-rc.1

[abhibeckert]

@sunaurus I think it would be good to learn from this and add some automated method to quickly deploy urgent security patches. There are thousands of lemmy instances and it will take forever for all of them to update.
I'm not suggesting fully automated software updates, just some way to quickly patch a critical vulnerability like this one.

Maybe with an opt-in/opt-out option because I'm sure some very specific admins will be up in arms about the potential abuse.

Sure... that option needs to exist, but it maybe don't make it too easy?

Automatically applying security patches is standard practice and the potential for abuse by a rogue Lemmy developer is vastly smaller than the potential abuse from a delayed installation.

If I was running a Lemmy instance, I would be defederating every instance that hasn't updated to v0.18.2.rc.1 until they do update. If instance admins want to, you know, sleep, then these patches need to be automated.

[kevincox]

What would defederation accomplish? The patch is in the client, patched servers can still send messages that trigger the bug. The problem is that the UI renders it insecurely.

Let's not get too excited. I think that there should be a designated channel for vulnerability announcements but I don't think we need to jump straight to automatically installing new code on servers. Without very careful design that can add worse vulnerabilities than it would prevent.

[abhibeckert]

What would defederation accomplish?

It would force those instance admins to update.

Also.. Admin and moderator accounts have been accessed on some of those servers. Login tokens for any (all?) other users may have also been compromised. And they have been posting links to illegal content.

The patch doesn't reset those JWT tokens either. So while the barn door is closed once instances update the wolf could be locked inside with the chickens.

[fkrauthan]

Also why should anyone install an RC version? Please release an official patch version. No one should have to install an RC to patch a bug.

[SineSwiper]

For future reference, this was patched by commit e80bcf5 and became available in version v0.18.2-rc.1

This is a serious enough issue to backport this to an official version.

[dessalines]

We plan on doing a release with a fix tomorrow, please be patient.

[geneccx]

We plan on doing a release with a fix tomorrow, please be patient.

@dessalines If it's not already on your radar, please also consider posting a GitHub security advisory on the issue for visibility, and also enabling private vulnerability reporting