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.

Sign up for GitHub

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

Jump to bottom

Lang cookie missing secure/httpOnly attributes #9690

Closed
2 tasks done
tgurr opened this issue Jan 10, 2020 · 5 comments · Fixed by #14279
Closed
2 tasks done

Lang cookie missing secure/httpOnly attributes #9690

tgurr opened this issue Jan 10, 2020 · 5 comments · Fixed by #14279
Labels
topic/security Something leaks user information or is otherwise vulnerable. Should be fixed!
Milestone
1.13.2

Comments

@tgurr
Copy link
Contributor

tgurr commented Jan 10, 2020

  • Gitea version: 1.10.2
  • Git version: 2.24.0
  • Operating system: Linux
  • Database:
    • MySQL
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)

gitea_cookies_trygiteaio

Description

I've configured my Gitea instance with CSRF_COOKIE_HTTP_ONLY and COOKIE_SECURE:

; Set false to allow JavaScript to read CSRF cookie
CSRF_COOKIE_HTTP_ONLY              = true
; If you use session in https only, default is false
COOKIE_SECURE     = true

which works fine, except for the lang cookie that doesn't seem to respect that setting:

gitea_cookies

resulting in a security scanner complaining about the cookie missing the secure and httpOnly attributes.
@lunny lunny added the topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! label Jan 10, 2020
@m-ueberall m-ueberall mentioned this issue Apr 11, 2020
Closed
7 tasks
@rodolpheh
Copy link

I think it all boils down to the i18n module for macaron-go : https://github.com/go-macaron/i18n/blob/5a5348d12f10480196f7376cf48de94fa93eb236/i18n.go#L202

@rodolpheh rodolpheh mentioned this issue Aug 13, 2020
Merged
@rodolpheh
Copy link

rodolpheh commented Aug 13, 2020 •
edited

My bad, I just saw that i18n in Gitea is a fork. Anyway, the changes I tried to introduce in go-macaron/i18n#11 works on the i18n version of Gitea. This was tested on a Docker build with latest sources.

image

@tgurr
Copy link
Contributor Author

tgurr commented Dec 30, 2020

@rodolpheh thanks for your work! Looks like the changes also have been pulled into the i18n Gitea fork in the meantime. Is there any option which allows to enable this or is currently the only way to enable/test this to manually change the settings which are off by default in i18n.go before compiling Gitea?

@rodolpheh
Copy link

Sorry I have a terrible memory and barely remembered doing that 😄 . Unfortunately it seems that I haven't pushed any changes that would allow setting those in Gitea. I had to dig a little before finding what I had changed in Gitea to make it work. Please find attached a patch file until a proper push is made on Gitea. I pulled the latest sources to make sure this works so hopefully it should work for you.

After applying this patch, the Secure and HttpOnly flags should be added if you had COOKIE_SECURE and CSRF_COOKIE_HTTP_ONLY set to true in your app settings.

macaron.go.patch.txt

@tgurr
Copy link
Contributor Author

tgurr commented Jan 4, 2021

@rodolpheh Thanks for the hint, I couldn't apply your patch, probably because I build from source so I had no /routers/routes/macaron.go b/routers/routes/macaron.go file to begin with.

Based on your patch I went with this: gitea-1.13.1-lang-cookie.txt. Not sure if it's ready for a PR like it is, but it appears to do the trick:

Screenshot_20210104_083856

@tgurr tgurr mentioned this issue Jan 7, 2021
Merged
6543 pushed a commit that referenced this issue Jan 7, 2021
@tgurr
Add secure/httpOnly attributes to the lang cookie (#9690) (#14279)
c1e30c9
6543 pushed a commit to 6543-forks/gitea that referenced this issue Jan 7, 2021
@tgurr @6543
Add secure/httpOnly attributes to the lang cookie (go-gitea#9690) (go…
d2bc459 
…-gitea#14279)
@6543 6543 added this to the 1.13.2 milestone Jan 7, 2021
@6543 6543 mentioned this issue Jan 7, 2021
Merged
6543 added a commit that referenced this issue Jan 7, 2021
@6543 @tgurr
Add secure/httpOnly attributes to the lang cookie (#14279) (#14280)
fb656b5 
* Add secure/httpOnly attributes to the lang cookie (#9690) (#14279)

* apply to InitLocales() too

Co-authored-by: Timo Gurr <timo.gurr@gmail.com>
a1012112796 added a commit to a1012112796/gitea that referenced this issue Jan 14, 2021
@a1012112796
Merge branch 'master' into imap
77cedeb 
* master: (252 commits)
  Issues overview should not show issues from archived repos (go-gitea#13220)
  Display SVG files as images instead of text (go-gitea#14101)
  [skip ci] Updated translations via Crowdin
  Update docs to clarify issues raised in go-gitea#14272 (go-gitea#14318)
  [skip ci] Updated translations via Crowdin
  [Refactor] Passwort Hash/Set (go-gitea#14282)
  Add option to change username to the admin panel (go-gitea#14229)
  fix mailIssueCommentBatch for pull request (go-gitea#14252)
  Remove self from MAINTAINERS (go-gitea#14286)
  Do not reload page after adding comments in Pull Request reviews (go-gitea#13877)
  Fix session bug when introduce chi (go-gitea#14287)
  [skip ci] Updated translations via Crowdin
  Add secure/httpOnly attributes to the lang cookie (go-gitea#9690) (go-gitea#14279)
  Some code improvements (go-gitea#14266)
  [skip ci] Updated translations via Crowdin
  Fix wrong type on hooktask to convert typ from char(16) to varchar(16) (go-gitea#14148)
  Upgrade XORM links in documentation. (go-gitea#14265)
  Check permission for the appropriate unit type (go-gitea#14261)
  Add compliance check for windows to ensure cross platform build (go-gitea#14260)
  [skip ci] Updated translations via Crowdin
  ...
@go-gitea go-gitea locked and limited conversation to collaborators Feb 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
topic/security Something leaks user information or is otherwise vulnerable. Should be fixed!
Projects
None yet
Milestone
1.13.2
Development

Successfully merging a pull request may close this issue.

Add secure/httpOnly attributes to the lang cookie (#9690)
4 participants
@lunny @tgurr @rodolpheh @6543