hugsy / ctfhub

Where CTFs happen
77 stars 13 forks source link

Insecure direct object reference #70

Closed h4ckd0tm3 closed 1 year ago

h4ckd0tm3 commented 1 year ago

Describe the bug The uploaded files are not renamed to "secure" filenames which is not an issue per se. However, in combination with the fact, that access to the files is unauthenticated leaves the application vulnerable to idor.

To Reproduce Steps to reproduce the behavior:

  1. Upload File
  2. Open Incognito Browser
  3. Copy File URL
  4. Watch the file getting downloaded

Additional context Debug is enabled which is the only way we can use any uploaded files at the moment at all. Because if we turn of Debug we are not even able to include the uploaded profile pictures as Django does not serve static files in production (with the exception of CSS and JS in the static folder).

Possible fix So for the avatars and stuff like that I don't really know to be honest. Maybe figure some nginx/apache setting out to let the webserver serve the files.

For the uploads we could use something like https://github.com/moggers87/django-sendfile2 in order to serve the files only to authenticated users.

EDIT: The app seems to not even serve the files that are in /static when in production mode. According to Django you have to configure the webserver to do that. https://docs.djangoproject.com/en/4.2/howto/static-files/deployment/

h4ckd0tm3 commented 1 year ago

For the avatars we could use whitenoise: https://github.com/evansd/whitenoise

EDIT: We cannot. whitenoise is apparently not meant to serve user uploaded content.

hugsy commented 1 year ago

I am aware of both points.CTFPad is not as mature as I'd like it to be, simply because I don't have the time to spend on it.

Regarding the deterministic uploaded file URLs, a teammate was preparing a PR to simply use the challenge ID as a directory name under uploads/. The path would still be predictable, but you'd have to know the 128-bit GUID first. Also a cheap addition would be make sure you're authenticated to be allowed to download.

Regarding the debug mode: yeah, static mode is a pain and since I'm adding features as we need them, I'm literally always in debug mode 😞 Here again, the same teammate suggested to move to django storages which could help, but again no time, no PR.

I'm welcoming any PR on either point if you feel like contributing more 😄

h4ckd0tm3 commented 1 year ago

Currently stuck with porting everything to Bootstrap 5.3.0 because it has a feature I want to use but after that I will look at it! U have quite a few things in backlog for this project already that I wanna open a PR for.

More than happy to contribute!

hugsy commented 1 year ago

Making a directory per challenge using the challenge GUID seems pretty trivial, I can get on that soon once my current update is done. For the rest, yeah as I said before I welcome contributions.

Quick word of warning, I'm considering moving the formatting to Black so there might be plenty of (small) changes coming soon.

h4ckd0tm3 commented 1 year ago

Making a directory per challenge using the challenge GUID seems pretty trivial, I can get on that soon once my current update is done. For the rest, yeah as I said before I welcome contributions.

Quick word of warning, I'm considering moving the formatting to Black so there might be plenty of (small) changes coming soon.

I fixed the IDOR in terms of challenge files, you are required to log in to download challenge files with my changes. I also implemented using the challenge GUID as folder in order to be able to find files better locally. If you for some reason have to search them on disk.

I will open a PR as soon as you merged #71 and #72

EDIT: With the changes, up/downloading files also works out of DEBUG mode.

hugsy commented 1 year ago

Closing since https://github.com/hugsy/ctfhub/pull/85 was merged with the changes