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] CSRF Vulnerability on API #4357

Closed
ghost opened this Issue Jul 3, 2018 · 6 comments

Comments

3 participants
@ghost

ghost commented Jul 3, 2018

  • Gitea version (or commit ref): every version with swagger api
  • 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

Screenshots

@lunny

This comment has been minimized.

Show comment
Hide comment
@lunny

lunny Jul 4, 2018

Member

Because Gitea's UI are also using some APIs, so the APIs support many authorizations(session is one of them). But have you found any missing permissions checking? Could you send a security email to security@gitea.io?

Member

lunny commented Jul 4, 2018

Because Gitea's UI are also using some APIs, so the APIs support many authorizations(session is one of them). But have you found any missing permissions checking? Could you send a security email to security@gitea.io?

@lunny

This comment has been minimized.

Show comment
Hide comment
@lunny

lunny Jul 6, 2018

Member

I think you are right. Maybe we should use different routes for UI and APIs.

Member

lunny commented Jul 6, 2018

I think you are right. Maybe we should use different routes for UI and APIs.

@techknowlogick

This comment has been minimized.

Show comment
Hide comment
@techknowlogick

techknowlogick Jul 6, 2018

Member

The way the WordPress API works is that if cookie auth is used (vs token auth), then a nonce has to be sent as well, the nonce is injected into the html template for JS to use and pass to the API and act as a second factor. (I've seen this approach in other implementations, it is just that WP is most well known)

Even if we did different routes for UI/API the UI ajax routes would still be affected.

More and more functionality is being built that uses the API (issue date, etc..) via the UI, and so we should work towards resolving the issue above.

Member

techknowlogick commented Jul 6, 2018

The way the WordPress API works is that if cookie auth is used (vs token auth), then a nonce has to be sent as well, the nonce is injected into the html template for JS to use and pass to the API and act as a second factor. (I've seen this approach in other implementations, it is just that WP is most well known)

Even if we did different routes for UI/API the UI ajax routes would still be affected.

More and more functionality is being built that uses the API (issue date, etc..) via the UI, and so we should work towards resolving the issue above.

@ghost ghost closed this Jul 6, 2018

@ghost ghost reopened this Jul 6, 2018

@techknowlogick

This comment has been minimized.

Show comment
Hide comment
@techknowlogick

techknowlogick Jul 7, 2018

Member

Perhaps we could use the CSRF token as a second factor

Member

techknowlogick commented Jul 7, 2018

Perhaps we could use the CSRF token as a second factor

@ghost ghost changed the title from [Security] API endpoint doesn't enforce an authorization to [Security] CSRF Vulnerability on API Jul 8, 2018

@ghost ghost referenced this issue Aug 2, 2018

Open

Gitea markdown not sanitised (links can contain API URLs) #4596

2 of 7 tasks complete
@techknowlogick

This comment has been minimized.

Show comment
Hide comment
@techknowlogick

techknowlogick Aug 9, 2018

Member

Anyone looking at this https://github.com/go-gitea/gitea/blob/master/routers/api/v1/api.go#L133 reqToken just validates that the user is signed in, not that they are using a valid token.

Member

techknowlogick commented Aug 9, 2018

Anyone looking at this https://github.com/go-gitea/gitea/blob/master/routers/api/v1/api.go#L133 reqToken just validates that the user is signed in, not that they are using a valid token.

@beeonthego

This comment has been minimized.

Show comment
Hide comment
@beeonthego

beeonthego Sep 1, 2018

Contributor

Is someone working on this issue now? if not, I will prepare a PR and check whether a token (or basic auth) has been used to authenticate. the check can be added to reqToken handler. There are a few minor changes, non breaking.

Contributor

beeonthego commented Sep 1, 2018

Is someone working on this issue now? if not, I will prepare a PR and check whether a token (or basic auth) has been used to authenticate. the check can be added to reqToken handler. There are a few minor changes, non breaking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment