Recovery middleware leaks sensitive cookies, tokens, and headers into logs

[dustin-decker]

Session cookies, API tokens, and authorization headers are all being leaked. The request is dumped if a handler hits a panic when using the recovery middleware:

gin/recovery.go

Line 41 in 65a65c2

httprequest, _ := httputil.DumpRequest(c.Request, false)

This is very insecure default behavior, and there appears to be no way to opt out other than not using the middleware.

[chainhelen]

@dustin-decker sorry, how do you define "leadked". This error is just logged in server, why leadked?

[dustin-decker]

It can make sensitive headers readable in plaintext logs. No production HTTP server I know of does this by default, though some can be configured to do this.
If you ship server logs somewhere for processing, aggregation, and viewing, then the problem becomes much more severe, and a lot of companies do this.

Twitter and Github recently publicly disclosed that they've accidentally logged passwords and advised people to change them: https://arstechnica.com/information-technology/2018/05/twitter-advises-users-to-reset-passwords-after-bug-posts-passwords-to-internal-log/

I will open a PR in a few minutes to address this issue in gin.

(Edited posts because I noticed that it does not log the body, but does log the other parts of the request)

[chainhelen]

@dustin-decker In my opinion, This is just a log middleware.

custom-middleware

func Logger() gin.HandlerFunc {
	return func(c *gin.Context) {
		t := time.Now()

		// Set example variable
		c.Set("example", "12345")

		// before request

		c.Next()

		// after request
		latency := time.Since(t)
		log.Print(latency)

		// access the status we are sending
		status := c.Writer.Status()
		log.Println(status)
	}
}

func main() {
	r := gin.New()
	r.Use(Logger())

	r.GET("/test", func(c *gin.Context) {
		example := c.MustGet("example").(string)

		// it would print: "12345"
		log.Println(example)
	})

	// Listen and serve on 0.0.0.0:8080
	r.Run(":8080")
}

I think it depend on developer，but not web framework.
En, just my opinion.Maybe you are right, wait for the explain of gin's member.

[appleboy]

ping @javierprovecho @thinkerou

[thinkerou]

Generally, we set username/password in body, as you say, we not log request body. If user set username/password in query param, user should encrypt self-user. As @dustin-decker pull request, remove log dump, I think it's ok, because we have had panic stack. This's my option.