Security Issue *DO NOT USE LTC* - Custom thumbnail_url feature

[wereii]

Overview

Since lemmy 0.19.4 (LemmyNet/lemmy#4425), users can provide their own thumbnail URL when creating a post.
Before 0.19.5 this was only available as (public) API option but since 0.19.5 this is also available to users through lemmy-ui. This feature is UI accessible since 0.19.4 too.
E: 0.19.4 has been released month and a week ago so this has been possible since then.

As is, this feature is not a problem [1] but combined with LTC it happens to fundamentally undermine how LTC works.

LTC assumes that the thumbnail URLs are internal (and always generated by lemmy/pictrs).
Now that the URL can be user provided we can't trust it.

Impact

Bad actors can abuse this to delete any local (in pictrs) image.

E: The broken URLs referenced in [1] do seem to allow for small but definitely possible attack surface to cause at best DoS, at worst SQLi.

POC

1. User edits old image post (old enough to pass as post to clean - THUMBNAIL_MIN_AGE_MONTHS)
2. Changes the thumbnail url to any localy hosted (in pictrs) image
3. Next time LTC checks the database, it will detect the old post that still has a thumbnail URL and process it for deletion, instructing pict-rs to delete the image - allowing the user to direct LTC to delete any local image.

Solutions

As far as we have checked, there is no way to absolutely differentiate between a user provided url (to local pictrs) and generated thumbnail (bar doing some heuristics on upload times and alike - possible to bypass).
Cross-checking with local_image is possible to stop deletion of any user uploaded images but does not solve it completely (deletion of lemmy generated images is still possible).

---

Thanks to @Nothing4You again for noticing this.

---

[1]: Except that the field allows some funny URLs e.g. https://t.t/;';'%22;...[:%3C%3E?]%27;%20yaba%20daba%20doo, if this is an issue too is not confirmed

DO NOT USE THIS PROJECT

Meanwhile watch this issue for any future information and possible resolution.

[poVoq]

Hmm, yeah IMHO this functionality should have been in core Lemmy from the start.

[dessalines]

Bad actors can abuse this to delete any local (in pictrs) image.

Isn't this only runnable by an instance admin anyway?

If you only want to delete thumbnails for local images, you can always scan the local_image table to find their pictrs urls.

[wereii]

Isn't this only runnable by an instance admin anyway?

Not sure I understand what you mean here - LTC is continually running in the background (as configured).

If you only want to delete thumbnails for local images, you can always scan the local_image table to find their pictrs urls.

Does this mean, instead of scanning posts for thumbnails to delete (and depend on thumbnail_url) it would read the local_images and if the image is old enough, ask pict-rs for all "derived" media (?) from that alias then delete that, something like that ?
This would ignore locally generated thumbnails for federated posts, correct?

[wereii]

Hmm, yeah IMHO this functionality should have been in core Lemmy from the start.
@poVoq

Yeah, some kind of simple metadata attached to each image in pict-rs would make this a lot easier, along {"reason": "upload|generated|..."}. Though would require extending pict-rs and probably changing how uploads work (but I am guessing here).

[Nutomic]

Bad actors can abuse this to delete any local (in pictrs) image.

Isn't this only runnable by an instance admin anyway?

@dessalines This tool runs continuously and deletes all thumbnails of posts which are over a month old. This is useful because Lemmy doesnt limit the thumbnail size and some of them are extremely large. Now the problem is, someone can set an image uploaded by another user as custom thumbnail for his own post, and then the thumbnail cleaner would delete that image because it doesnt know that its used elsewhere.

To avoid this problem we could add a column in Lemmy like post.has_custom_thumbnail, then thumbnail cleaner could skip those. Or store this same info in pictrs as suggested by @wereii.

Hmm, yeah IMHO this functionality should have been in core Lemmy from the start.

That wont help, Lemmy currently doesnt store any info whether a thumbnail is custom or auto-generated. What we could do is limit the size of post thumbnails (eg 150x150 px) so they take much less space.

Edit: A possible solution would be for ltc to check before deleting any thumbnail that it isnt used elsewhere. Ie query all the fields post.url, post.body, avatars etc if they contain the same link as the thumbnail to delete. If so, skip the item. This would require the same logic as #3.

[dessalines]

To avoid this problem we could add a column in Lemmy like post.has_custom_thumbnail

This should def be a part of the image table rework I need to do.