From ded02a8857d2d092a490f5f394ebd4094023e9bf Mon Sep 17 00:00:00 2001 From: motoyasu-saburi Date: Sat, 1 Apr 2023 12:25:32 +0900 Subject: [PATCH 1/3] fix lack of escaping of filename in Content-Disposition --- context.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/context.go b/context.go index 5716318e1f..4d87c32683 100644 --- a/context.go +++ b/context.go @@ -1056,7 +1056,7 @@ func (c *Context) FileFromFS(filepath string, fs http.FileSystem) { // On the client side, the file will typically be downloaded with the given filename func (c *Context) FileAttachment(filepath, filename string) { if isASCII(filename) { - c.Writer.Header().Set("Content-Disposition", `attachment; filename="`+filename+`"`) + c.Writer.Header().Set("Content-Disposition", `attachment; filename="`+strings.Replace(filename, "\"", "\\\"", -1)+`"`) } else { c.Writer.Header().Set("Content-Disposition", `attachment; filename*=UTF-8''`+url.QueryEscape(filename)) } From 96c3c556fc200ffe2dd4d9ab5a480d2a455c2560 Mon Sep 17 00:00:00 2001 From: motoyasu-saburi Date: Sat, 1 Apr 2023 12:36:57 +0900 Subject: [PATCH 2/3] add test for Content-Disposition filename escaping process --- context_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/context_test.go b/context_test.go index 1dec902c69..54e6dacb7d 100644 --- a/context_test.go +++ b/context_test.go @@ -1032,6 +1032,20 @@ func TestContextRenderAttachment(t *testing.T) { assert.Equal(t, fmt.Sprintf("attachment; filename=\"%s\"", newFilename), w.Header().Get("Content-Disposition")) } +func TestContextRenderAndEscapeAttachment(t *testing.T) { + w := httptest.NewRecorder() + c, _ := CreateTestContext(w) + maliciousFilename := "tampering_field.sh\";dummy=.go" + actualEscapedResponseFilename := "tampering_field.sh\\\";dummy=.go" + + c.Request, _ = http.NewRequest("GET", "/", nil) + c.FileAttachment("./gin.go", maliciousFilename) + + assert.Equal(t, 200, w.Code) + assert.Contains(t, w.Body.String(), "func New() *Engine {") + assert.Equal(t, fmt.Sprintf("attachment; filename=\"%s\"", actualEscapedResponseFilename), w.Header().Get("Content-Disposition")) +} + func TestContextRenderUTF8Attachment(t *testing.T) { w := httptest.NewRecorder() c, _ := CreateTestContext(w) From ee5b2c95e53395c03102e95b479b48d67dfee9c8 Mon Sep 17 00:00:00 2001 From: motoyasu-saburi Date: Sat, 13 May 2023 19:42:43 +0900 Subject: [PATCH 3/3] fix filename escape bypass problem fix backslashes before backquotes were not properly escaped problem. --- context.go | 8 +++++++- context_test.go | 4 ++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/context.go b/context.go index 4d87c32683..cb360879c6 100644 --- a/context.go +++ b/context.go @@ -1052,11 +1052,17 @@ func (c *Context) FileFromFS(filepath string, fs http.FileSystem) { http.FileServer(fs).ServeHTTP(c.Writer, c.Request) } +var quoteEscaper = strings.NewReplacer("\\", "\\\\", `"`, "\\\"") + +func escapeQuotes(s string) string { + return quoteEscaper.Replace(s) +} + // FileAttachment writes the specified file into the body stream in an efficient way // On the client side, the file will typically be downloaded with the given filename func (c *Context) FileAttachment(filepath, filename string) { if isASCII(filename) { - c.Writer.Header().Set("Content-Disposition", `attachment; filename="`+strings.Replace(filename, "\"", "\\\"", -1)+`"`) + c.Writer.Header().Set("Content-Disposition", `attachment; filename="`+escapeQuotes(filename)+`"`) } else { c.Writer.Header().Set("Content-Disposition", `attachment; filename*=UTF-8''`+url.QueryEscape(filename)) } diff --git a/context_test.go b/context_test.go index 54e6dacb7d..180512356d 100644 --- a/context_test.go +++ b/context_test.go @@ -1035,8 +1035,8 @@ func TestContextRenderAttachment(t *testing.T) { func TestContextRenderAndEscapeAttachment(t *testing.T) { w := httptest.NewRecorder() c, _ := CreateTestContext(w) - maliciousFilename := "tampering_field.sh\";dummy=.go" - actualEscapedResponseFilename := "tampering_field.sh\\\";dummy=.go" + maliciousFilename := "tampering_field.sh\"; \\\"; dummy=.go" + actualEscapedResponseFilename := "tampering_field.sh\\\"; \\\\\\\"; dummy=.go" c.Request, _ = http.NewRequest("GET", "/", nil) c.FileAttachment("./gin.go", maliciousFilename)