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

server-side request forgery (SSRF) vulnerability in webhooks #4624

Closed
2 of 7 tasks
Siesh1oo opened this issue Aug 6, 2018 · 9 comments
Closed
2 of 7 tasks

server-side request forgery (SSRF) vulnerability in webhooks #4624

Siesh1oo opened this issue Aug 6, 2018 · 9 comments
Labels
topic/security Something leaks user information or is otherwise vulnerable. Should be fixed!

Comments

@Siesh1oo
Copy link

Siesh1oo commented Aug 6, 2018

  • Gitea version (or commit ref): 1.5.0+rc1-98-g9ea327f1f
  • Git version: not relevant
  • Operating system: not relevant
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No
    • Not relevant
  • Log gist:

Description

Due to shared code base, gitea is affected by issue gogs/gogs#5366 (server-side request forgery (SSRF) vulnerability in webhooks).

To reproduce:

  • create Webhook with target URL to any running existing service exposed only to localhost (for example localhost:8080 or localhost:19999, depending on what is running on the test machine).
  • POST content

Screenshots

See gogs/gogs#5366.

@lafriks lafriks added the topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! label Aug 6, 2018
@SagePtr
Copy link
Contributor

SagePtr commented Aug 6, 2018

Probably there should be IP/subnet blacklist which could be configured in ini file or via admin panel.
But this functionality should be configurable because in some situations webhook may point to server on the same machine.
Could be also checkbox which allows to bypass protection for certain webhooks, but this checkbox should be visible and editable by server administrators only (like regular hooks) and auto-reset to off if webhook url is modified by non-admin user.

@Siesh1oo
Copy link
Author

Siesh1oo commented Aug 7, 2018

webhook URLs should never be allowed to point to localhost, 127.0.0.1, ::1, server name, server IP address, etc (one might consider an isAdmin() exception tho)

@techknowlogick
Copy link
Member

The code that needs to get changed is here: https://github.com/go-gitea/gitea/blob/master/modules/httplib/httplib.go#L315-L339

Some similar code can be found here: https://github.com/hakobe/paranoidhttp/blob/master/client.go#L109

A note that Dial is deprecated, and should be changed to DialContext

@tosone
Copy link

tosone commented Aug 15, 2018

Default IP black list should block the local network, and admin can change this by hand.

@lafriks
Copy link
Member

lafriks commented Aug 15, 2018

I think it needs some kind of admin setting in app.ini to disable that

@daviian
Copy link
Member

daviian commented Aug 23, 2018

I love the idea to limit webhooks using internal ip address range to admin permission since the admin has knowledge about internal server infrastructure / services.
So if a project / repository needs that they can request an admin to add this webhook.

@Siesh1oo
Copy link
Author

@daviian : still non-Admin User sind should probably not be allowed to create Webhooks pointing to the local server, or is there some use case where this seems useful (and harmless?)?

@daviian
Copy link
Member

daviian commented Aug 28, 2018

@Siesh1oo yes you're right. non-admin users shouldn't be allowed to add such webhooks. if they need to, for whatever reason, they should be required to ask an admin to do so.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Jul 5, 2022

I have fixed it by introducing allow list

@lunny lunny closed this as completed Jul 5, 2022
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
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

No branches or pull requests

8 participants