From 988bb32ec1aed3bbab72b309f6a56b7dbd081d89 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 9 Jun 2021 15:15:41 +0800 Subject: [PATCH 1/5] Fix http path bug --- integrations/git_smart_http_test.go | 69 +++++++++++++++++++++++++++++ routers/web/repo/http.go | 19 ++++++++ routers/web/web.go | 2 +- 3 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 integrations/git_smart_http_test.go diff --git a/integrations/git_smart_http_test.go b/integrations/git_smart_http_test.go new file mode 100644 index 000000000000..a81c63d1ae0b --- /dev/null +++ b/integrations/git_smart_http_test.go @@ -0,0 +1,69 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package integrations + +import ( + "io/ioutil" + "net/http" + "net/url" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestGitSmartHTTP(t *testing.T) { + onGiteaRun(t, testGitSmartHTTP) +} + +func testGitSmartHTTP(t *testing.T, u *url.URL) { + var kases = []struct { + p string + code int + }{ + /*{ + p: "info/refs", + code: 200, + },*/ + { + p: "user2/repo1/HEAD", + code: 200, + }, + { + p: "user2/repo1/objects/info/alternates", + code: 404, + }, + { + p: "user2/repo1/objects/info/http-alternates", + code: 404, + }, + { + p: "user2/repo1/../../custom/conf/app.ini", + code: 404, + }, + { + p: "user2/repo1/objects/info/../../../../custom/conf/app.ini", + code: 404, + }, + { + p: `user2/repo1/objects/info/..\..\..\..\custom\conf\app.ini`, + code: 400, + }, + } + + for _, kase := range kases { + t.Run(kase.p, func(t *testing.T) { + p := u.String() + kase.p + req, err := http.NewRequest("GET", p, nil) + assert.NoError(t, err) + req.SetBasicAuth("user2", userPassword) + resp, err := http.DefaultClient.Do(req) + assert.NoError(t, err) + defer resp.Body.Close() + assert.EqualValues(t, kase.code, resp.StatusCode) + _, err = ioutil.ReadAll(resp.Body) + assert.NoError(t, err) + }) + } +} diff --git a/routers/web/repo/http.go b/routers/web/repo/http.go index 30d382b8ef18..4ec5000da9fb 100644 --- a/routers/web/repo/http.go +++ b/routers/web/repo/http.go @@ -366,7 +366,26 @@ func (h *serviceHandler) setHeaderCacheForever() { h.w.Header().Set("Cache-Control", "public, max-age=31536000") } +func containsDotDot(v string) bool { + if !strings.Contains(v, "..") { + return false + } + for _, ent := range strings.FieldsFunc(v, isSlashRune) { + if ent == ".." { + return true + } + } + return false +} + +func isSlashRune(r rune) bool { return r == '/' || r == '\\' } + func (h *serviceHandler) sendFile(contentType, file string) { + if containsDotDot(file) { + log.Error("request file path contains invalid path: %v", file) + h.w.WriteHeader(http.StatusBadRequest) + return + } reqFile := path.Join(h.dir, file) fi, err := os.Stat(reqFile) diff --git a/routers/web/web.go b/routers/web/web.go index 6c0141eef30a..4686370d4424 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1000,7 +1000,7 @@ func RegisterRoutes(m *web.Route) { m.Get("/objects/info/alternates", repo.GetTextFile("objects/info/alternates")) m.Get("/objects/info/http-alternates", repo.GetTextFile("objects/info/http-alternates")) m.Get("/objects/info/packs", repo.GetInfoPacks) - m.Get("/objects/info/{file:[^/]*}", repo.GetTextFile("")) + m.Get("/objects/info/{file:*}", repo.GetTextFile("")) m.Get("/objects/{head:[0-9a-f]{2}}/{hash:[0-9a-f]{38}}", repo.GetLooseObject) m.Get("/objects/pack/pack-{file:[0-9a-f]{40}}.pack", repo.GetPackFile) m.Get("/objects/pack/pack-{file:[0-9a-f]{40}}.idx", repo.GetIdxFile) From 9fddf3347302f80c17e5d90f164edd48e24e1c98 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 9 Jun 2021 15:17:42 +0800 Subject: [PATCH 2/5] Add missed request --- integrations/git_smart_http_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/integrations/git_smart_http_test.go b/integrations/git_smart_http_test.go index a81c63d1ae0b..9a4e3689c1ce 100644 --- a/integrations/git_smart_http_test.go +++ b/integrations/git_smart_http_test.go @@ -22,10 +22,10 @@ func testGitSmartHTTP(t *testing.T, u *url.URL) { p string code int }{ - /*{ - p: "info/refs", + { + p: "user2/repo1/info/refs", code: 200, - },*/ + }, { p: "user2/repo1/HEAD", code: 200, From b5493a27f7699468d213e25651bddd9a600dc6f1 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 9 Jun 2021 15:18:52 +0800 Subject: [PATCH 3/5] revert unnecessary change --- routers/web/web.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/web/web.go b/routers/web/web.go index 4686370d4424..6c0141eef30a 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1000,7 +1000,7 @@ func RegisterRoutes(m *web.Route) { m.Get("/objects/info/alternates", repo.GetTextFile("objects/info/alternates")) m.Get("/objects/info/http-alternates", repo.GetTextFile("objects/info/http-alternates")) m.Get("/objects/info/packs", repo.GetInfoPacks) - m.Get("/objects/info/{file:*}", repo.GetTextFile("")) + m.Get("/objects/info/{file:[^/]*}", repo.GetTextFile("")) m.Get("/objects/{head:[0-9a-f]{2}}/{hash:[0-9a-f]{38}}", repo.GetLooseObject) m.Get("/objects/pack/pack-{file:[0-9a-f]{40}}.pack", repo.GetPackFile) m.Get("/objects/pack/pack-{file:[0-9a-f]{40}}.idx", repo.GetIdxFile) From 1a32539f3b977bc4d2e16a521691683e1db7d3e8 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 9 Jun 2021 14:08:48 +0200 Subject: [PATCH 4/5] add Unit test & rename --- routers/web/repo/http.go | 4 ++-- routers/web/repo/http_test.go | 39 +++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 routers/web/repo/http_test.go diff --git a/routers/web/repo/http.go b/routers/web/repo/http.go index 4ec5000da9fb..649d6d1eb1a3 100644 --- a/routers/web/repo/http.go +++ b/routers/web/repo/http.go @@ -366,7 +366,7 @@ func (h *serviceHandler) setHeaderCacheForever() { h.w.Header().Set("Cache-Control", "public, max-age=31536000") } -func containsDotDot(v string) bool { +func containsParentDirectorySeparator(v string) bool { if !strings.Contains(v, "..") { return false } @@ -381,7 +381,7 @@ func containsDotDot(v string) bool { func isSlashRune(r rune) bool { return r == '/' || r == '\\' } func (h *serviceHandler) sendFile(contentType, file string) { - if containsDotDot(file) { + if containsParentDirectorySeparator(file) { log.Error("request file path contains invalid path: %v", file) h.w.WriteHeader(http.StatusBadRequest) return diff --git a/routers/web/repo/http_test.go b/routers/web/repo/http_test.go new file mode 100644 index 000000000000..fdbd5eb3eb93 --- /dev/null +++ b/routers/web/repo/http_test.go @@ -0,0 +1,39 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package repo + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestContainsParentDirectorySeparator(t *testing.T) { + tests := []struct { + v string + b bool + }{ + { + v: `user2/repo1/info/refs`, + b: false, + }, + { + v: `user2/repo1/HEAD`, + b: false, + }, + { + v: `user2/repo1/../../custom/conf/app.ini`, + b: true, + }, + { + v: `user2/repo1/objects/info/..\..\..\..\custom\conf\app.ini`, + b: true, + }, + } + + for i := range tests { + assert.EqualValues(t, tests[i].b, containsParentDirectorySeparator(tests[i].v)) + } +} From f954bb3820f9baddd313edf5cdf4fd55b13d4de6 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 9 Jun 2021 14:11:42 +0200 Subject: [PATCH 5/5] add strange file case --- routers/web/repo/http_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/routers/web/repo/http_test.go b/routers/web/repo/http_test.go index fdbd5eb3eb93..58ac1c07a129 100644 --- a/routers/web/repo/http_test.go +++ b/routers/web/repo/http_test.go @@ -23,6 +23,10 @@ func TestContainsParentDirectorySeparator(t *testing.T) { v: `user2/repo1/HEAD`, b: false, }, + { + v: `user2/repo1/some.../strange_file...mp3`, + b: false, + }, { v: `user2/repo1/../../custom/conf/app.ini`, b: true,