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.

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

fixed open redirect #3907

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

cyal1
Copy link

@cyal1 cyal1 commented Mar 28, 2024

Hi,

In the default configuration, Gin enables RedirectTrailingSlash, which allows paths with trailing slashes to work properly even in /:param1/:param2 pattern, for example /hostname/evil.com/.

This is achieved by redirecting users to /hostname/evil.com using the redirectTrailingSlash function. However, if the param1 is empty, users are redirected to //evil.com, browsers will automatically prepend the http(s) protocol for url starting with //. Below is a simple vulnerable code snippet.

package main

import (
 "net/http"

 "github.com/gin-gonic/gin"
)

func main() {
 router := gin.Default()

 router.GET("/", func(c *gin.Context) {
  c.JSON(http.StatusOK, gin.H{
   "message": "Hello Gin!",
  })
 })

 router.GET("/:param1/:param2", func(c *gin.Context) {
  param1 := c.Param("param1")
  param2 := c.Param("param2")

  c.JSON(http.StatusOK, gin.H{
   "param1":  param1,
   "param2": param2,
  })

 })

 router.Run(":8080")
}

You can observe the request being redirected to a malicious website by accessing http://localhost:8080//evil.com/ in your browser or using the curl command curl -v http://localhost:8080//evil.com/. This issue affects versions 1.9.1 and earlier.

image image

Below is the fix code:

func redirectTrailingSlash(c *Context) {
	req := c.Request
	p := req.URL.Path
	if prefix := path.Clean(c.Request.Header.Get("X-Forwarded-Prefix")); prefix != "." {
		prefix = regSafePrefix.ReplaceAllString(prefix, "")
		prefix = regRemoveRepeatedChar.ReplaceAllString(prefix, "/")

		p = prefix + "/" + req.URL.Path
	}
	req.URL.Path = p + "/"
	p = regexp.MustCompile("^/{2,}").ReplaceAllString(p, "/") // fix open redirect
	if length := len(p); length > 1 && p[length-1] == '/' {
		req.URL.Path = p[:length-1]
	}
	redirectRequest(c)
}

@appleboy
Copy link
Member

appleboy commented Apr 1, 2024

@cyal1 Please add testing case.

@appleboy appleboy added the bug label Apr 1, 2024
@cyal1
Copy link
Author

cyal1 commented Apr 1, 2024

@cyal1 Please add testing case.

Hello, I have added the test case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants