Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assets which are unused will remain on the server #635

Open
Kee7702 opened this issue Jan 12, 2023 · 9 comments
Open

Assets which are unused will remain on the server #635

Kee7702 opened this issue Jan 12, 2023 · 9 comments
Labels
bug Something isn't working server:website

Comments

@Kee7702
Copy link

Kee7702 commented Jan 12, 2023

Describe the bug
When something like a profile picture is uploaded, the old asset isn't deleted. This essentially means that there is a potential for exploits, by rapidly uploading new images.

To Reproduce
Steps to reproduce the behavior:

  1. Use inspect element to find the image url to your current profile picture, and open the image in a new tab.
  2. Go to upload a custom profile picture either on the website or any of the LBP games.
  3. Refresh the old profile picture, and it'll still be on the server.

Expected behavior
After maybe a few minutes or days it would've been deleted, but it remains on the server.

Screenshots
No screenshot, but here's an example. Both /gameAssets/42663ec9c449f964cef4a0fb3b0653ce74c098fe and /gameAssets/a1e4ed63b74a5285958b19a1491142130f476346 were uploaded to the same profile, and 1 of which should now go unused and thus would be wasting storage space.

Game(s) affected

  • This issue likely affects all platforms but I am unsure, as this issue can only be observed through the website.

Additional context
I had to reupload my profile picture from LBP1 since uploading from the website caused a placeholder image to show on LBP1. Since this affects profile pictures, it is also likely to affect things like shared images and level badges.

@Kee7702 Kee7702 added bug Something isn't working server:gameserver labels Jan 12, 2023
@m88youngling
Copy link
Contributor

Great find! Thank you for documenting this!

@Kee7702
Copy link
Author

Kee7702 commented Jan 12, 2023

Great find! Thank you for documenting this!

It's definitely a vulnerability for when this will become a public server, and it was discovered due to a bug. LBP1 did not properly load the image, but for why I'm not sure. Might it be required to convert file formats depending on the game connected to the server?

@sudokoko
Copy link
Member

However, storing unused assets would be good for moderation. Would be quite easy to catch anyone uploading inappropriate content. However storing that off-site would be a better way to do this.

@m88youngling
Copy link
Contributor

Good idea. I was told by a rather sus character back in 2021 that this was what the official servers did. Perhaps we could set up a config entry where you can plug in a link for where deleted assets are shipped off to. But, that still doesn't solve the problem of controls to prevent abuse.

One tool already in motion that can help prevent abuse is server-side rate limiting, which is outside the scope of Lighthouse but would in theory protect against people maliciously uploading assets.

@sudokoko
Copy link
Member

sudokoko commented Jan 12, 2023

Good idea. I was told by a rather sus character back in 2021 that this was what the official servers did. Perhaps we could set up a config entry where you can plug in a link for where deleted assets are shipped off to. But, that still doesn't solve the problem of controls to prevent abuse.

One tool already in motion that can help prevent abuse is server-side rate limiting, which is outside the scope of Lighthouse but would in theory protect against people maliciously uploading assets.

You made a good point here. Potentially a limit to how many times you can update your profile picture per 24 hours, a longer-timed version of how Discord does it.

As for deleted asset storage, having configuration options for something like an S3 Bucket would probably work. Will look into it.

@Slendy
Copy link
Contributor

Slendy commented Jan 12, 2023

The current functionality of lighthouse is that when a photo is deleted its corresponding resources are also deleted. What can happen though is if that photo was used in other places it can be re-uploaded through other means like publishing levels or setting a profile icon.
Edit: I just noticed that this issue is in reference to the custom profile picture feature on the website which doesn't delete old photos

@Kee7702
Copy link
Author

Kee7702 commented Jan 13, 2023

The current functionality of lighthouse is that when a photo is deleted its corresponding resources are also deleted. What can happen though is if that photo was used in other places it can be re-uploaded through other means like publishing levels or setting a profile icon. Edit: I just noticed that this issue is in reference to the custom profile picture feature on the website which doesn't delete old photos

Specifically, this issue isn't just in relation to the website. I only found out because I uploaded it through LBP1 due to the file uploaded from the website not loading on LBP1(which is the only issue here relating to the website). It is only observable on the website because I can actually get a file path. The issue also could be useful in some cases, so it definitely should remain an option to keep old assets, but to actually associate them to a user account that can be viewed by a moderator.

@TorutheRedFox
Copy link
Contributor

However, storing unused assets would be good for moderation. Would be quite easy to catch anyone uploading inappropriate content. However storing that off-site would be a better way to do this.

garbage collecting old files could be done too

a way of doing this would be touching the files when a file that's dependent on it gets uploaded or deleted, then checking their modification date upon garbage collection, while keeping a log of dependencies that should be updated as resources dependent on others are uploaded and deleted, with unused and deleted files, which should show whether the file is unused or not, and should be deleted if the last event involving it was more than, say 30 days ago (make it admin configurable) and it's unused

@sudokoko
Copy link
Member

Touching back on this, could also be a maintenance routine via the admin panel that can be run at the instance admin's descretion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working server:website
Projects
None yet
Development

No branches or pull requests

5 participants