c.Redirect(http.StatusTemporaryRedirect, "") leads to open redirect vulnerability

[franciscoescher]

Description

c.Redirect(http.StatusTemporaryRedirect, "") can lead to open redirect vulnerability

How to reproduce

package main

import (
	"net/http"

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

func main() {
	g := gin.Default()
	g.GET("/home", func(c *gin.Context) {
		c.String(200, "Hello %s", c.Param("name"))
	})
	g.NoRoute(func(c *gin.Context) {
		c.Redirect(http.StatusTemporaryRedirect, "")
	})
	g.Run(":9000")
}

Expectations

Open your browser in http://localhost:9000///%5Cwww.google.com/
I would expect to receive a 404 not found page

Actual result

It leads me to google instead (or any other malicious url)

Environment

• go version: 1.22
• gin version (or commit ref): 1.10
• operating system: osx sonoma 1.45

[imvtsl]

I analyzed the issue. Here is the analysis:

1- First, we call "func (c *Context) Redirect(code int, location string)". According to documentation, this method "Redirect returns an HTTP redirect to the specific location." This statement in documentation sounds misleading as I will explain later.

2- This method creates a render.Redirect object. In this object, request field is set to context.Request. This contains "GET ///\www.google.com/". After creating this object, it passes this object to "func (c *Context) Render(code int, r render.Render)".

3- Render function of context calls "func (r Redirect) Render(w http.ResponseWriter)". The parameter w is nothing but context.Writer. context.writer contains "GET ///\www.google.com/".

4- This function internally calls http.redirect function. According to documentation of net/http package, "Redirect replies to the request with a redirect to url, which may be a path relative to the request path."

So, the http.Redirect function interprets the URL (which is an empty string in our case) as relative to the request path "///[www.google.com/](http://www.google.com/)" in the code below:

if url == "" || url[0] != '/' { // make relative path absolute olddir, _ := path.Split(oldpath) url = olddir + url }

In this scenario, the url becomes "///\www.google.com/".

Referring back to the first point, the Gin documentation's statement is misleading because it suggests that the function redirects to the specified location. However, as we can see, it may redirect to a URL that is a path relative to the request path.
I believe we should fix documentation.

Regarding the open redirect vulnerability, it seems to be caused by user implementation. Users should follow best practices as listed in the OWASP Unvalidated Redirects and Forwards Cheat Sheet.

@appleboy
Can I proceed and change the documentation as above?

[RedCrazyGhost]

The solution I can think of so far：

1. When Location is an empty string, we can directly panic in the redirect method to ensure that we don't bring this problem online. This is a simpler solution.
2. Whether it is possible to submit a redirect method using IRouter as a parameter based on this proposal？proposal: add redirect to a specific handler #789

[imvtsl]

@RedCrazyGhost
I can't think of a use case where we would want redirect location as an empty string (In relative terms, it would mean the request url itself). I think 1st solution would be good.

I think we should also modify the documentation for Redirect function in Gin to state that it may redirect to to a URL that is a path relative to the request path.

@appleboy your thoughts here?

[monguis]

@RedCrazyGhost I believe that solution num 2 is out of this issue's scope, besides it is already proposed.

I would assume that num 1 would take care of this, being the logical answer IMHO.