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

[SECURITY] Static Links to uploaded files #986

Closed
ybelenko opened this issue May 28, 2019 · 7 comments

Comments

@ybelenko
Copy link
Contributor

commented May 28, 2019

Feature Request

About audited Directus version.
It has been cloned from suite repo.
Latest commit directus/directus@1d151a9

Description:

In Internet systems, there is sometimes a file upload mechanism, which allowing the user to upload files to share information between entities that communicate through the application. Files upload mechanism often generate direct links to download the uploaded files from the system. These mechanisms sometimes choose a name of file and store the uploaded file according to the name the chosen on the web server. A mechanism that defines file names in a sequence numbers is an insecure mechanism because it allows guessing of uploaded files more easily, thus allowing access to sensitive personal information.

Business risk:

After exploiting this vulnerability, an attacker can easily guess links to uploaded files.

Technical details:

During the test, it was found that the application’s upload files mechanism generates static links to uploaded files, thing that allows an attacker to get access to uploaded files that were uploaded into the system, even if were uploaded by other users.

screenshot

What problem does this feature solve?

Fixes security hole.

How do you think this should be implemented?

  • Do not let users to get access to upload files that were uploaded by another users.
  • Image ID should be chosen by unpredictable way and one-time random token should be appended to the
    link in order to prevent exact link guessing.
  • Implement an authorization mechanism that validate if user authorized to view or download the specific file.

Would you be willing to work on this?

Maybe, with help/guidance from Directus team.

@benhaynes benhaynes added this to Needs triage in Bug Triage via automation May 31, 2019

@bjgajjar bjgajjar moved this from Needs triage to Medium priority in Bug Triage Jun 6, 2019

@itsmerhp

This comment has been minimized.

Copy link
Contributor

commented Jun 22, 2019

@benhaynes @rijkvanzanten

As we are going to implement UUID feature for file uploading system, guessing the file name is near to impossible.
Eventhough, if we want to restrict accessing files for unauthorised user, we have to restrict directly accessing of files and create functionality which will check if a user is authorised to access the file or not. To achieve this we need to make changes in system where files are being fetched / accessed.

Please share your thoughts on it.

@benhaynes

This comment has been minimized.

Copy link
Member

commented Jun 24, 2019

Is this an exact duplicate of #987 ?

My comment is the same as on that one.

@shealavington

This comment has been minimized.

Copy link
Member

commented Jun 24, 2019

This should be resolved by adding UUID's, the short version of this post is if you see /image-1.jpg, more than likely, you could guess that there's going to be a /image-2.jpg

@bjgajjar bjgajjar added the duplicate label Jun 26, 2019

@bjgajjar

This comment has been minimized.

Copy link
Member

commented Jun 26, 2019

It will be fixed with the development of #337

@bjgajjar bjgajjar marked this as a duplicate of #337 Jun 26, 2019

@rijkvanzanten

This comment has been minimized.

Copy link
Member

commented Jun 26, 2019

We need to pay attention to the subtle difference between this and #987 though. This issue states

During the test, it was found that the application’s upload files mechanism generates static links to uploaded files, thing that allows an attacker to get access to uploaded files that were uploaded into the system, even if were uploaded by other users.

By using UUIDs, this will not be fixed, as it's still a public static link.

The business risk will be fixed though:

After exploiting this vulnerability, an attacker can easily guess links to uploaded files.

As the UUID is way harder / impossible to guess. That being said, a UUID is not a hash or any other cryptographically secure method of generating file names, so one could theoretically work out the UUID structure and programmatically "guess" filenames.

@bjgajjar

@kaushal-im

This comment has been minimized.

Copy link
Member

commented Jun 27, 2019

@rijkvanzanten You are right, with UUID, we will be only able to achieve the solution so that attacker can't guess the file names.
But still, the unauthorized directory access will be available until we push the fixes for #987 which will be preventing unauthorized access to the files of different users.

@bjgajjar @itsmerhp You can push the fix for #987 and it will automatically fix this issue.

@bjgajjar

This comment has been minimized.

Copy link
Member

commented Jul 2, 2019

Agreed with @kaushal-im. It will be fixed with the development of #987. So closing this issue. :)

@bjgajjar bjgajjar closed this Jul 2, 2019

Bug Triage automation moved this from Medium priority to Closed Jul 2, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
7 participants
You can’t perform that action at this time.