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

attachments accessible without authorization #4721

Closed
2 of 7 tasks
noerw opened this issue Aug 15, 2018 · 5 comments · Fixed by #6481
Closed
2 of 7 tasks

attachments accessible without authorization #4721

noerw opened this issue Aug 15, 2018 · 5 comments · Fixed by #6481
Labels
topic/security Something leaks user information or is otherwise vulnerable. Should be fixed!
Milestone

Comments

@noerw
Copy link
Member

noerw commented Aug 15, 2018

Description

Attachments of a release on a private repository should not be accessible without authorization!

For example check the following private repo: https://try.gitea.io/norwin/test/releases
The release has an attachment, which can be accessed without authentication via
https://try.gitea.io/attachments/f3763540-6bf8-47c2-b2ce-0fa9c48f1e82

You could argue that the UUID provides security, but I would definitely feel safer if the attachments were protected by the same ACLs that govern the code-tarball associated with each release in case the direct link leaks somewhere.

@lafriks lafriks added the topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! label Aug 15, 2018
@leepfrog-ger
Copy link

You could argue that the UUID provides security

There are many cases where links might get leaked to another party (e.g. browser addon, some tool reading the download history, using untrusted chat services, proxy server...).
So I agree that this should be treated as security issue

@cs8425
Copy link

cs8425 commented Mar 31, 2019

The root cause:

m.Post("/attachments", repo.UploadAttachment)
}, ignSignIn)

We should add some verifying for m.Post("/attachments", repo.UploadAttachment).
Where are we using POST to /attachments?
Upload attachments for repo/issue/comment only?
At least needs login for sure.

this issue existed at gogs too:
gogs/gogs#5599
https://github.com/gogs/gogs/blob/f545faa06d553750b9f4018336e810530389f88c/cmd/web.go#L338-L340

@adelowo
Copy link
Member

adelowo commented Mar 31, 2019

To be fair, even images uploaded in a private repo on GitHub are accessible without authorization.

I think another issue is tracking this same bug as I must have made the above comment somewhere before

@cs8425
Copy link

cs8425 commented Apr 1, 2019

@adelowo
I mean that anyone can upload without authorization.
I use the same method which gogs issue reported, and with a little modify, it still works.

  1. get CSRF token by curl -s https://try.gitea.io/ | grep csrf
  2. make a multipart/form-data
--boundary
Content-Disposition: form-data; name="file"; filename="test.txt"
Content-Type: image/png

<here are binary data from a png file>
--boundary--
  1. upload data by: curl -kis -X 'POST' -H "X-Csrf-Token: $CSRF" -H 'Content-Type: multipart/form-data; boundary=boundary' -b "lang=en-US; _csrf=$CSRF" --data-binary @req.txt 'https://try.gitea.io/attachments'
  2. got {"uuid":"93fdc72f-a2e2-4bbb-8930-07105c29513f"}
  3. check the file: https://try.gitea.io/attachments/93fdc72f-a2e2-4bbb-8930-07105c29513f

And I found that if remove the file from here:
test
There are no any http request to notify server to delete it.
Both may cause DoS attack for disk space.
First one is easy to fix, just set request login before upload.
But the second need more work, we need to check who can delete the uploaded file.
Or I should create a new issue for this?

@chotaire
Copy link

chotaire commented Apr 7, 2019

Tested as working in Gitea Version: 1.9.0+dev-61-g7ed65a98e.

@lafriks lafriks added this to the 1.9.0 milestone Apr 8, 2019
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
topic/security Something leaks user information or is otherwise vulnerable. Should be fixed!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants