X-Forwarded-For handling is still unsafe, CVE-2020-28483 is NOT fixed

[rwos]

Description

X-Forwarded-For/ trusted proxy handling is incorrect, which makes it possible for anyone to force the value of c.ClientIP(), if:

• the app has trusted proxies defined
• and the trusted proxy handles X-Forwarded-For in the usual way, by appending IP addresses at the end

(the default configuration trusts every proxy and is of course also vulnerable, in a very trivial way).

This was reported in #2473 with a fix at #2474. That PR did not get merged, and the one that did (#2632) does not fix the issue.

There is a fix for this already at #2844.

How to reproduce

You actually have that in your tests already, see https://github.com/gin-gonic/gin/pull/2844/files#diff-e6ce689a25eaef174c2dd51fe869fabbe04a6c6afbd416b23eda138c82e761baR1432

But here's a standalone version

package main

import (
	"fmt"
	"net/http"
	"net/http/httptest"

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

func main() {
	r := gin.Default()
	r.TrustedProxies = []string{"1.1.1.1"}
	r.GET("/", func(c *gin.Context) { c.String(200, "ClientIP: %s", c.ClientIP()) })
	r.Run("XXXX") // to work around https://github.com/gin-gonic/gin/pull/2692 not being released yet

	req, _ := http.NewRequest("GET", "/", nil)

	// client IP: 7.7.7.7, as reported by 6.6.6.6 (which is untrusted)
	// all of this proxied through 1.1.1.1 (which is trusted)
	req.Header.Set("X-Forwarded-For", "7.7.7.7, 6.6.6.6")
	req.RemoteAddr = "1.1.1.1:1234"

	w := httptest.NewRecorder()
	r.ServeHTTP(w, req)

	// This should print 6.6.6.6, which is the last IP a trusted proxy
	// actually connected to. But it prints 7.7.7.7, which can be spoofed
	// by the completly untrusted 6.6.6.6
	fmt.Println(w.Body.String())
}

Expectations

$ go run main.go
ClientIP: 6.6.6.6

Actual result

$ go run main.go
ClientIP: 7.7.7.7

Environment

• go version: 1.17
• gin version (or commit ref): 1.7.4
• operating system: macOS

[Bisstocuz]

Yep, this issue should be resolved, otherwise all the works did before are wasted.

[adamwoolhether]

still waiting for this.

[Bisstocuz]

Hi, #2844 was merged, I think this issue can be closed.

[till]

@Bisstocuz Maybe a release would help.

[agiammar]

When we can expect this issue to be closed and a new release with the fix?

[mrgadgil]

I am using the latest release v1.7.4, our Image vulnerability scanner complains about CVE-2020-28483`` Is the CVE-2020-28483` fixed? From the pull request it looks like the CVE-2020-28483 is fixed. Want to know if it is just a matter of creating a new release?

[appleboy]

I will bump the new release tag v1.7.5 and close this issue, maybe today or tommorow.

[mrgadgil]

@appleboy Thank you for the quick response. Will wait for the new release tag v1.7.5

[mrgadgil]

@appleboy When can we expect the release to be created? Our service is blocked on the release since we cannot move ahead in our pipeline as the image vulnerability acts as a blocker in our pipeline. Request for the release to be created asap. Thank you.

[appleboy]

@mrgadgil Sorry for the late reply. @thinkerou Do you have time to handle it? Or maybe I will take it tomorrow or the day after tomorrow.

[vanimi]

Hello. When can we expect the release to be created?

[mrgadgil]

@appleboy @Bisstocuz Updated the gin-gonic/gin version to 1.7.6, but IBM image vulnerability scanner marks [CVE-2020-28483] in the image. Can you please look?

[kszafran]

The 1.7.6 version is the same as 1.7.4. I believe it was released only to cover up some issue with the 1.7.5 release.

[mrgadgil]

@kszafran Thank you. Just checked 1.7.4, 1.7.5 and 1.7.6 are all at the same commit id.
@appleboy Can you please create a release which has fix for #2862 We have been waiting for over a week now. Appreciate if you can create a release.

[thinkerou]

v1.7.7 have released, thanks! https://github.com/gin-gonic/gin/releases