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

Closed
opened 2025-11-02 04:25:02 -06:00 by GiteaMirror · 11 comments
Owner

Originally created by @Siesh1oo on GitHub (Aug 2, 2018).

  • Gitea version (or commit ref): 1.5.0+rc1-94-g819f50ccd
  • Git version: nevermind
  • Operating system: nevermind
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io: probably (currently down "bad gateway")
    • Yes (provide example URL)
    • No
    • Not relevant
  • Log gist:

Description

Create README.md or other markdown file containing a link to some gitea API endpoint:

[I might be a misleading, yet harmless looking link. Click me to learn more](/user/logout)

Problem

API endpoints are normal GET requests, not protected by auth (normal session id used).

Markdown is rendered in normal (non-sandboxed) DOM.

Potential solutions

  1. sanitize URLs (do not allow relative or absolute links to API endpoints; if so, regex search'n'replace can for example fill with zero-width/invisible unicode whitespace, or just strike it through),
  2. sandbox markdown rendered DOM in iframe with sandbox attribute set (and remove session cookie from outside using JavaScript),
  3. Enforce CSRF for all API endpoints.

ideally all three

Originally created by @Siesh1oo on GitHub (Aug 2, 2018). - Gitea version (or commit ref): 1.5.0+rc1-94-g819f50ccd - Git version: nevermind - Operating system: nevermind - Database (use `[x]`): - [ ] PostgreSQL - [x] MySQL - [ ] MSSQL - [ ] SQLite - Can you reproduce the bug at https://try.gitea.io: probably (currently down "bad gateway") - [ ] Yes (provide example URL) - [ ] No - [x] Not relevant - Log gist: ## Description Create README.md or other markdown file containing a link to some gitea API endpoint: ``` [I might be a misleading, yet harmless looking link. Click me to learn more](/user/logout) ``` ## Problem API endpoints are normal GET requests, not protected by auth (normal session id used). Markdown is rendered in normal (non-sandboxed) DOM. ## Potential solutions 1. sanitize URLs (do not allow relative or absolute links to API endpoints; if so, regex search'n'replace can for example fill with zero-width/invisible unicode whitespace, or just strike it through), 2. sandbox markdown rendered DOM in iframe with sandbox attribute set (and remove session cookie from outside using JavaScript), 3. Enforce CSRF for all API endpoints. ideally all three
GiteaMirror added the topic/security label 2025-11-02 04:25:02 -06:00
Author
Owner

@Siesh1oo commented on GitHub (Aug 2, 2018):

Addendum

Same bug for images. Note that the user does not even has to click the image, loading the page will execute the injected API code.

Testcase: create README.md containing an image with URL to API endpoint, log in, visit repository page, do something else and realise that gitea has logged you out:

![This is not an Image of Yaktocat](/user/logout)
@Siesh1oo commented on GitHub (Aug 2, 2018): ## Addendum Same bug for images. Note that the user does not even has to click the image, loading the page will execute the injected API code. Testcase: create README.md containing an image with URL to API endpoint, log in, visit repository page, do something else and realise that gitea has logged you out: ``` ![This is not an Image of Yaktocat](/user/logout) ```
Author
Owner

@lafriks commented on GitHub (Aug 2, 2018):

Yes this should be cleaned up but no harming actions can be done as GET requests are only for getting data not doing actions (except for logout url)

@lafriks commented on GitHub (Aug 2, 2018): Yes this should be cleaned up but no harming actions can be done as GET requests are only for getting data not doing actions (except for logout url)
Author
Owner

@Siesh1oo commented on GitHub (Aug 3, 2018):

@lafriks: Are you sure? The team/org router go suggested otherwise; logout was used above only as harmless example

@Siesh1oo commented on GitHub (Aug 3, 2018): @lafriks: Are you sure? The team/org router go suggested otherwise; logout was used above only as harmless example
Author
Owner

@Siesh1oo commented on GitHub (Aug 3, 2018):

Maybe some collaboration possible: https://github.com/gogs/gogs/issues/5355#issuecomment-410117204

@Siesh1oo commented on GitHub (Aug 3, 2018): Maybe some collaboration possible: https://github.com/gogs/gogs/issues/5355#issuecomment-410117204
Author
Owner

@Siesh1oo commented on GitHub (Aug 3, 2018):

@cezar97:

I complicated myself in trying to add this image that send a GET request in the repository's description. I should added it in the Markdown like you did 😄.

The two issues seem distinct, would need distinct fixes, if solved via sanitizing as discussed there: removing/sanitizing HTML tags from markdown wouldn't fix this one. Markdown links need special treatment unless URLs and images are completely forbidden, which is probably not desirable

@Siesh1oo commented on GitHub (Aug 3, 2018): @cezar97: > I complicated myself in trying to add this image that send a GET request in the repository's description. I should added it in the Markdown like you did 😄. The two issues seem distinct, would need distinct fixes, if solved via sanitizing as discussed there: removing/sanitizing HTML tags from markdown wouldn't fix this one. Markdown links need special treatment unless URLs and images are completely forbidden, which is probably not desirable
Author
Owner

@msg7086 commented on GitHub (Aug 4, 2018):

I'd say, actions that can change system states, such as logout or star, shall be POST only.

@msg7086 commented on GitHub (Aug 4, 2018): I'd say, actions that can change system states, such as logout or star, shall be POST only.
Author
Owner

@Siesh1oo commented on GitHub (Aug 4, 2018):

@msg7086: it‘s about the missing CSRF protection token, and that user-provided content is executed/rendered in the same DOM context as the gitea API calls.

CSRF on GET is technically possible, albeit sometimes considered bad practice (one might argue that token is a bit less exposed when passed in body instead of header).

Unfortunately it seems that the macaron CSRF checker does not support CSRF on GET (is this correct? I might be wrong here).

Sending post from simple link URL is possible via jquery $.post(...).

This could possibly be used as minimally invasive hotfix: Replace all GET API actions fav/watch/logout/org.action/etc by href="javascript:$.post(url, CSRF-token-string)", and do not accept GET anywhere in the gitea/router except for landing pages and repo-over-http.

@Siesh1oo commented on GitHub (Aug 4, 2018): @msg7086: it‘s about the missing CSRF protection token, and that user-provided content is executed/rendered in the same DOM context as the gitea API calls. CSRF on GET is technically possible, albeit sometimes considered bad practice (one might argue that token is a bit less exposed when passed in body instead of header). Unfortunately it seems that the macaron CSRF checker does not support CSRF on GET (is this correct? I might be wrong here). Sending post from simple link URL is possible via jquery $.post(...). This could possibly be used as minimally invasive hotfix: Replace all GET API actions fav/watch/logout/org.action/etc by href="javascript:$.post(url, CSRF-token-string)", and do not accept GET anywhere in the gitea/router except for landing pages and repo-over-http.
Author
Owner

@msg7086 commented on GitHub (Aug 4, 2018):

Thanks for pointing out. Yes CSRF is also needed in this particular case. Just that my reply was focusing on best practices about HTTP verbs (e.g. RESTful style) and not about XSS protection.

Cheers

@msg7086 commented on GitHub (Aug 4, 2018): Thanks for pointing out. Yes CSRF is also needed in this particular case. Just that my reply was focusing on best practices about HTTP verbs (e.g. RESTful style) and not about XSS protection. Cheers
Author
Owner

@Siesh1oo commented on GitHub (Aug 6, 2018):

@lafriks : related, with example: https://github.com/gogs/gogs/issues/5367

@Siesh1oo commented on GitHub (Aug 6, 2018): @lafriks : related, with example: https://github.com/gogs/gogs/issues/5367
Author
Owner

@Siesh1oo commented on GitHub (Aug 13, 2018):

One more to check, this one had affected gitlab too:

server-side request forgery (SSRF) vulnerability in migrate https://github.com/gogs/gogs/issues/5372

@Siesh1oo commented on GitHub (Aug 13, 2018): One more to check, this one had affected gitlab too: > server-side request forgery (SSRF) vulnerability in migrate https://github.com/gogs/gogs/issues/5372
Author
Owner

@jolheiser commented on GitHub (Mar 3, 2020):

Fixed by #10462, #10465, #10582

@jolheiser commented on GitHub (Mar 3, 2020): Fixed by #10462, #10465, #10582
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/gitea#2134