Add escape logic for header

[t0rchwo0d]

Add escape logic for header

Hi, Team.
I added a escape logic to the header reflecting user input values for the following.
I'd appreciate it if you could review it.

Description

Basically X-Forwarded-Prefix is not required for any purpose other than the / delimiter. However, unintended execution by crafted request.

X-Forwarded-Prefix has a potential problems. Although actively exploiting this flaw is unlikely, Need to prevents abuse in scenarios such as cache poisoning.

How to reproduce

package main

import (
	"net/http"

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

func main() {

	r := gin.Default()

	r.GET("/bug", func(c *gin.Context) {
		c.JSON(http.StatusBadRequest, gin.H{"msg": "bug"})
	})

	r.Run()
}

Case 1. Modulate

Expectations

$ curl -L --path-as-is -i "http://192.168.1.19:8080/bug/" -H "x-forwarded-prefix: ../../bug#?"

HTTP/1.1 301 Moved Permanently
Content-Type: text/html; charset=utf-8
Location: ../../../bug%2523%253F/bug
Date: Tue, 14 Feb 2023 17:17:42 GMT
Content-Length: 61

HTTP/1.1 404 Not Found
Content-Type: text/plain
Date: Tue, 14 Feb 2023 17:17:42 GMT
Content-Length: 18

404 page not found

Actual result

$ curl -L --path-as-is -i "http://192.168.1.19:8080/bug/" -H "x-forwarded-prefix: ../../bug#?"

HTTP/1.1 301 Moved Permanently
Content-Type: text/html; charset=utf-8
Location: ../../bug#?//../../bug%23%3F//bug
Date: Tue, 14 Feb 2023 17:10:43 GMT
Content-Length: 68

HTTP/1.1 400 Bad Request
Content-Type: application/json; charset=utf-8
Date: Tue, 14 Feb 2023 17:10:43 GMT
Content-Length: 13

{"msg":"bug"}

Case 2. Redirect

Expectations

$ curl -L --path-as-is -i "http://192.168.1.19:8080/bug/" -H "x-forwarded-prefix: https://gin-gonic.com/#"

HTTP/1.1 301 Moved Permanently
Content-Type: text/html; charset=utf-8
Location: https%3A/gin-gonic.com/%23/https%253A/gin-gonic.com/%2523/bug
Date: Tue, 14 Feb 2023 17:16:27 GMT
Content-Length: 96

HTTP/1.1 404 Not Found
Content-Type: text/plain
Date: Tue, 14 Feb 2023 17:16:27 GMT
Content-Length: 18

404 page not found

Actual result

$ curl -L --path-as-is -i "http://192.168.1.19:8080/bug/" -H "x-forwarded-prefix: https://gin-gonic.com/#"

HTTP/1.1 301 Moved Permanently
Content-Type: text/html; charset=utf-8
Location: https:/gin-gonic.com/#/https:/gin-gonic.com/%23/bug
Date: Tue, 14 Feb 2023 17:13:50 GMT
Content-Length: 86

HTTP/2 200 
server: GitHub.com
content-type: text/html; charset=utf-8
last-modified: Fri, 29 Apr 2022 07:17:57 GMT
access-control-allow-origin: *
etag: "626b9125-6605"
expires: Tue, 14 Feb 2023 17:23:50 GMT
cache-control: max-age=600
x-proxy-cache: MISS
x-github-request-id: 77D4:63EE:1951BF:1A6FCE:63EBC14E
accept-ranges: bytes
date: Tue, 14 Feb 2023 17:13:50 GMT
via: 1.1 varnish
age: 0
x-served-by: cache-tyo11949-TYO
x-cache: MISS
x-cache-hits: 0
x-timer: S1676394830.054868,VS0,VE181
vary: Accept-Encoding
x-fastly-request-id: a7775619919d2344b41c5b64e35a19b5f1db36c0
content-length: 26117

# ... skip ...

Case 3. Infinite Loop

Expectations

$ curl -L --path-as-is -i "http://192.168.1.19:8080/bug/" -H "x-forwarded-prefix: #bug"

HTTP/1.1 301 Moved Permanently
Content-Type: text/html; charset=utf-8
Location: %23bug/%2523bug/bug
Date: Tue, 14 Feb 2023 17:19:00 GMT
Content-Length: 54

HTTP/1.1 404 Not Found
Content-Type: text/plain
Date: Tue, 14 Feb 2023 17:19:00 GMT
Content-Length: 18

404 page not found

Actual result

$ curl -L --path-as-is -i "http://192.168.1.19:8080/bug/" -H "x-forwarded-prefix: #bug"

HTTP/1.1 301 Moved Permanently
Content-Type: text/html; charset=utf-8
Location: #bug/%23bug/bug
Date: Tue, 14 Feb 2023 17:20:09 GMT
Content-Length: 50

# ... Loop ...

HTTP/1.1 301 Moved Permanently
Content-Type: text/html; charset=utf-8
Location: #bug/%23bug/bug
Date: Tue, 14 Feb 2023 17:20:19 GMT
Content-Length: 50

curl: (47) Maximum (50) redirects followed

Environment

• go version: go version go1.19 windows/amd64
• gin version (or commit ref): latest
• operating system: windows

Reference

• GHSA-6qq8-5wq3-86rp
• GHSA-6vm3-jj99-7229
• https://cwe.mitre.org/data/definitions/20.html
• https://cwe.mitre.org/data/definitions/601.html
• https://cwe.mitre.org/data/definitions/835.html
• Add mitigation for log injection #2237

[codecov]

Codecov Report

Merging #3500 (cb5832c) into master (c1d06e3) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #3500   +/-   ##
=======================================
  Coverage   98.63%   98.63%           
=======================================
  Files          42       42           
  Lines        3148     3151    +3     
=======================================
+ Hits         3105     3108    +3     
  Misses         29       29           
  Partials       14       14           

Flag	Coverage Δ
	98.63% <100.00%> (+<0.01%)	⬆️
go-1.16	∅ <ø> (∅)
go-1.17	98.54% <100.00%> (+<0.01%)	⬆️
go-1.18	98.54% <100.00%> (+<0.01%)	⬆️
go-1.19	98.63% <100.00%> (+<0.01%)	⬆️
go-1.20	98.63% <100.00%> (+<0.01%)	⬆️
macos-latest	98.54% <100.00%> (-0.10%)	⬇️
ubuntu-latest	98.63% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
gin.go	99.19% <100.00%> (+<0.01%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[appleboy]

@t0rchwo0d testing fail. Please take a look.

[t0rchwo0d]

Hi, @appleboy
Thank you for your review.

Hmm, It's stuck down there.
macos-latest @ Go 1.19 -tags "sonic avx"

=== RUN   TestPathCleanMallocs
    path_test.go:85: 
        	Error Trace:	/Users/runner/work/gin/gin/path_test.go:85
        	Error:      	Not equal: 
        	            	expected: float64(3837)
        	            	actual  : int(0)
        	Test:       	TestPathCleanMallocs

I think it's related to the following.
Looks like probabilistic error occur.
#2596

Can you plz start testing again?

[t0rchwo0d]

Update the same code for build test

[t0rchwo0d]

@appleboy Oh, testing passed!
I'd appreciate it if you could review it again.

[thinkerou]

please add some unit test cases, thanks!

[t0rchwo0d]

Hi, @thinkerou
Thank you for your review!

I update "routes_test.go", Plz check again : )

[t0rchwo0d]

update "routes_test.go

[t0rchwo0d]

Hi, @appleboy, @thinkerou

Sorry, There was a lack of additional filter consideration.
So, I have opened the following.
#3503