Gitea markdown not sanitised (links can contain API URLs)

[Siesh1oo]

• 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

[Siesh1oo]

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)

[lafriks]

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)

[Siesh1oo]

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

[Siesh1oo]

Maybe some collaboration possible: gogs/gogs#5355 (comment)

[Siesh1oo]

@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

[msg7086]

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

[Siesh1oo]

@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.

[msg7086]

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

[Siesh1oo]

@lafriks : related, with example: gogs/gogs#5367

[Siesh1oo]

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

server-side request forgery (SSRF) vulnerability in migrate gogs/gogs#5372

[jolheiser]

Fixed by #10462, #10465, #10582