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

Some Small Issues For Consideration. #3

Open
Quintensen opened this issue Jan 16, 2025 · 5 comments
Open

Some Small Issues For Consideration. #3

Quintensen opened this issue Jan 16, 2025 · 5 comments
Assignees

Comments

@Quintensen
Copy link

Below are some issue I found upon reviewing your code.
Disclaimer: I do not write in golang or I would have provided some examples or pull requests to fix these. There also could be some code I did not understand and I saw an issue that you had already thought of and implemented.

1. Three character file names
One of the issues with this is that the files can be easily bruteforcable and allow for others to skim your uploads dir and bog down your server. Another issue is that one upload could conflict with another. User1 uploads file1 and gets named abc user2 uploads file2 and gets named abc. User1 goes to download file1 using the link to abc and downloads file2. The fix for this would be to use GUIDs so that the chances of file overwrite and brute-forcing is basically impossible(if implemented correctly)

2. When the file is uploaded the file name is lost.
file1.txt is changed to abc upon uploading the file.
The fix for this would be to use a GUID to create a directory and store the file in there. This would keep the filename and increase security as the bruteforcer would need the GUID and the filename.

3. Files will not be deleted if the server restarts.
The server will start a timer to delete the given file after X amount of time. If the server is restarted the files will no longer be scheduled for deletion.
The fix for this would be to have a file that either logs the creation time of a file or the scheduled deletion time of a file and then when the server is started/after a set period of time while running it will check that file and determine if the file has expired and needs to be deleted.

4. X-Forwarded-For can be anything
Your logging code will log the X-Forwarded-For header without checking if it is a valid IP address.
The fix for this would be to validate the header is an IP address or to log both the header and the IP the HTTP request is coming from.

5. The server will redirect to the file immediately after uploading.
This can be confusing for the end user and be annoying for them as well.
I recommend having another HTML file that the user is redirected to that has the file URL and a download button.

6. Unlimited file uploads
Currently you have the server hosted publicly and I dont see any way the server is stopping someone from uploading junk data to fill your HDD or upload multiple 1GB files that are fragmented from a larger file to bypass limits.

I like the project and I hope you take these issues into consideration for the next release.

@ImnotEdMateo ImnotEdMateo self-assigned this Jan 16, 2025
@ImnotEdMateo
Copy link
Owner

Thank you for your Feedback!

I'll work in it.

@ImnotEdMateo ImnotEdMateo pinned this issue Jan 16, 2025
@ImnotEdMateo
Copy link
Owner

ImnotEdMateo commented Jan 17, 2025

Well, in theory, I have solved the first commit, now the path to be generated is configurable, you can put an integer to generate something bigger than 3 characters or put GUID to generate a GUID...

You can see the commit here

@ImnotEdMateo
Copy link
Owner

Ok, I've solved issue No. 2, finally the file names are not lost, I implemented the system that creates the directories for each upload, I didn't like the idea of ​​having such a long url (you know, guid/file), I thought it would be better to have more accessible urls... well you can see the commit here

@Quintensen
Copy link
Author

Looks good so far :) (although I am kinda bad at reading commits and git in general) :)

@ImnotEdMateo
Copy link
Owner

issue No. 5 solved!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants