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

Prevent redirect to Host (2) #19175

Merged
merged 6 commits into from
Mar 23, 2022
Merged

Prevent redirect to Host (2) #19175

merged 6 commits into from
Mar 23, 2022

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented Mar 22, 2022 •
edited
Loading

Unhelpfully Locations starting with /\ will be converted by the
browser to // because ... well I do not fully understand. Certainly
the RFCs and MDN do not indicate that this would be expected. Providing
"compatibility" with the (mis)behaviours of a certain proprietary OS is
my suspicion. However, we clearly have to protect against this.

Therefore we should reject redirection locations that match the regular
expression: ^/[\\\\/]+

Reference #9678

Signed-off-by: Andrew Thornton art27@cantab.net

@zeripath zeripath added this to the 1.17.0 milestone Mar 22, 2022
@zeripath
Prevent redirect to Host (2)
8e97a93 
Unhelpfully Locations starting with `/\` will be converted by the
browser to `//` because ... well I do not fully understand. Certainly
the RFCs and MDN do not indicate that this would be expected. Providing
"compatibility" with the (mis)behaviour of a certain proprietary OS is
my suspicion. However, we clearly have to protect against this.

Therefore we should reject redirection locations that match the regular
expression: `^/[\\\\/]+`

Reference go-gitea#9678

Signed-off-by: Andrew Thornton <art27@cantab.net>
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 22, 2022
@zeripath
simplify
ca59d88 
Signed-off-by: Andrew Thornton <art27@cantab.net>
wxiaoguang
wxiaoguang approved these changes Mar 23, 2022
View reviewed changes
modules/context/context.go Show resolved Hide resolved
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 23, 2022
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 23, 2022
@zeripath zeripath merged commit 0b1686b into go-gitea:main Mar 23, 2022
@zeripath zeripath deleted the prevent-redirect-to-host-2 branch March 23, 2022 16:12
zeripath added a commit to zeripath/gitea that referenced this pull request Mar 23, 2022
@zeripath
Prevent redirect to Host (2) (go-gitea#19175)
1e3d3f6 
Backport go-gitea#19175

Unhelpfully Locations starting with `/\` will be converted by the
browser to `//` because ... well I do not fully understand. Certainly
the RFCs and MDN do not indicate that this would be expected. Providing
"compatibility" with the (mis)behaviour of a certain proprietary OS is
my suspicion. However, we clearly have to protect against this.

Therefore we should reject redirection locations that match the regular
expression: `^/[\\\\/]+`

Reference go-gitea#9678

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this pull request Mar 23, 2022
Merged
@zeripath zeripath added the backport/done All backports for this PR have been created label Mar 23, 2022
zeripath added a commit that referenced this pull request Mar 23, 2022
@zeripath
Prevent redirect to Host (2) (#19175) (#19186)
e3d8e92 
Backport #19175

Unhelpfully Locations starting with `/\` will be converted by the
browser to `//` because ... well I do not fully understand. Certainly
the RFCs and MDN do not indicate that this would be expected. Providing
"compatibility" with the (mis)behaviour of a certain proprietary OS is
my suspicion. However, we clearly have to protect against this.

Therefore we should reject redirection locations that match the regular
expression: `^/[\\\\/]+`

Reference #9678

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath added the topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! label Mar 23, 2022
zeripath added a commit to zeripath/gitea that referenced this pull request Mar 23, 2022
@zeripath
Changelog for 1.16.5
fb7de1c 
 ## [1.16.5](https://github.com/go-gitea/gitea/releases/tag/1.16.5) - 2022-03-23

* BREAKING
  * Bump to build with go1.18 (go-gitea#19120 et al) (go-gitea#19127)
* SECURITY
  * Prevent redirect to Host (2) (go-gitea#19175) (go-gitea#19186)
  * Try to prevent autolinking of displaynames by email readers (go-gitea#19169) (go-gitea#19183)
  * Clean paths when looking in Storage (go-gitea#19124) (go-gitea#19179)
  * Do not send notification emails to inactive users (go-gitea#19131) (go-gitea#19139)
  * Do not send activation email if manual confirm is set (go-gitea#19119) (go-gitea#19122)
* ENHANCEMENTS
  * Use the new/choose link for New Issue on project page (go-gitea#19172) (go-gitea#19176)
* BUGFIXES
  * Fix compare link in active feeds for new branch (go-gitea#19149) (go-gitea#19185)
  * Redirect .wiki/* ui link to /wiki (go-gitea#18831) (go-gitea#19184)
  * Ensure deploy keys with write access can push (go-gitea#19010) (go-gitea#19182)
  * Ensure that setting.LocalURL always has a trailing slash (go-gitea#19171) (go-gitea#19177)
  * Cleanup protected branches when deleting users & teams (go-gitea#19158) (go-gitea#19174)
  * Use IterateBufferSize whilst querying repositories during adoption check (go-gitea#19140) (go-gitea#19160)
  * Fix NPE /repos/issues/search when not signed in (go-gitea#19154) (go-gitea#19155)
  * Use custom favicon when viewing static files if it exists (go-gitea#19130) (go-gitea#19152)
  * Fix the editor height in review box (go-gitea#19003) (go-gitea#19147)
  * Ensure isSSH is set whenever DISABLE_HTTP_GIT is set (go-gitea#19028) (go-gitea#19146)
  * Fix wrong scopes caused by empty scope input (go-gitea#19029) (go-gitea#19145)
  * Make migrations SKIP_TLS_VERIFY apply to git too (go-gitea#19132) (go-gitea#19141)
  * Handle email address not exist (go-gitea#19089) (go-gitea#19121)
* MISC
  * Update json-iterator to allow compilation with go1.18 (go-gitea#18644) (go-gitea#19100)
  * Update golang.org/x/crypto (go-gitea#19097) (go-gitea#19098)

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this pull request Mar 23, 2022
Merged
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 24, 2022
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
851c6ca 
* giteaofficial/main:
  Bump minimist from 1.2.5 to 1.2.6 (go-gitea#19194)
  Changelog for 1.16.5 (go-gitea#19189) (go-gitea#19192)
  Fix showing issues in your repositories (go-gitea#18916)
  Update issue_no_dependencies description (go-gitea#19112)
  Prevent redirect to Host (2) (go-gitea#19175)
  Prevent start panic due to missing DotEscape function
  Fix compare link in active feeds for new branch (go-gitea#19149)
  Redirect .wiki/* ui link to /wiki (go-gitea#18831)
  Try to prevent autolinking of displaynames by email readers (go-gitea#19169)
  Update HTTP status codes to modern codes (go-gitea#18063)
Chianina pushed a commit to Chianina/gitea that referenced this pull request Mar 28, 2022
@zeripath
Prevent redirect to Host (2) (go-gitea#19175)
b84abc0 
Unhelpfully Locations starting with `/\` will be converted by the
browser to `//` because ... well I do not fully understand. Certainly
the RFCs and MDN do not indicate that this would be expected. Providing
"compatibility" with the (mis)behaviour of a certain proprietary OS is
my suspicion. However, we clearly have to protect against this.

Therefore we should reject redirection locations that match the regular
expression: `^/[\\\\/]+`

Reference go-gitea#9678

Signed-off-by: Andrew Thornton <art27@cantab.net>
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kdumontnu kdumontnu kdumontnu left review comments

@wxiaoguang wxiaoguang wxiaoguang approved these changes

@lafriks lafriks lafriks approved these changes

Assignees
No one assigned
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! type/bug
Projects
None yet
Milestone
1.17.0
Development

Successfully merging this pull request may close these issues.
5 participants
@zeripath @lafriks @wxiaoguang @kdumontnu @GiteaBot