From 4ca2780dbf19d137746041886525fdebe594e50a Mon Sep 17 00:00:00 2001 From: runner361 Date: Sun, 29 May 2022 07:54:48 +0800 Subject: [PATCH] Fix issue 4961 Fix issue 4961, `leafInfo.match()` use `path.join()` to deal with `wildcardValues`, which may lead to cross directory risk --- CHANGELOG.md | 2 +- server/web/tree.go | 2 ++ server/web/tree_test.go | 11 ++++++++++- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0075ddcf..3745e2b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,5 @@ # developing - +- [Fix issue 4961, `leafInfo.match()` use `path.join()` to deal with `wildcardValues`, which may lead to cross directory risk ](https://github.com/beego/beego/pull/4964) # v2.0.3 - [upgrade redisgo to v1.8.8](https://github.com/beego/beego/pull/4872) - [fix prometheus CVE-2022-21698](https://github.com/beego/beego/pull/4878) diff --git a/server/web/tree.go b/server/web/tree.go index 3e0b86c5..adcb8853 100644 --- a/server/web/tree.go +++ b/server/web/tree.go @@ -282,6 +282,8 @@ func (t *Tree) addseg(segments []string, route interface{}, wildcards []string, // Match router to runObject & params func (t *Tree) Match(pattern string, ctx *context.Context) (runObject interface{}) { + // fix issue 4961, deal with "./ ../ //" + pattern = path.Clean(pattern) if pattern == "" || pattern[0] != '/' { return nil } diff --git a/server/web/tree_test.go b/server/web/tree_test.go index f43c4a87..af0c888e 100644 --- a/server/web/tree_test.go +++ b/server/web/tree_test.go @@ -68,7 +68,8 @@ func init() { matchTestInfo("/", "/", nil), matchTestInfo("/customer/login", "/customer/login", nil), matchTestInfo("/customer/login", "/customer/login.json", map[string]string{":ext": "json"}), - matchTestInfo("/*", "/http://customer/123/", map[string]string{":splat": "http://customer/123/"}), + // This case need to be modified when fix issue 4961, "//" will be replaced with "/" and last "/" will be deleted before route. + matchTestInfo("/*", "/http://customer/123/", map[string]string{":splat": "http:/customer/123"}), matchTestInfo("/*", "/customer/2009/12/11", map[string]string{":splat": "customer/2009/12/11"}), matchTestInfo("/aa/*/bb", "/aa/2009/bb", map[string]string{":splat": "2009"}), matchTestInfo("/cc/*/dd", "/cc/2009/11/dd", map[string]string{":splat": "2009/11"}), @@ -125,6 +126,14 @@ func init() { // test for fix of issue 4946 notMatchTestInfo("/suffix/:name", "/suffix.html/suffix.html"), matchTestInfo("/suffix/:id/name", "/suffix/1234/name.html", map[string]string{":id": "1234", ":ext": "html"}), + // test for fix of issue 4961,path.join() lead to cross directory risk + matchTestInfo("/book1/:name/fixPath1/*.*", "/book1/name1/fixPath1/mybook/../mybook2.txt", map[string]string{":name": "name1", ":path": "mybook2"}), + notMatchTestInfo("/book1/:name/fixPath1/*.*", "/book1/name1/fixPath1/mybook/../../mybook2.txt"), + notMatchTestInfo("/book1/:name/fixPath1/*.*", "/book1/../fixPath1/mybook/../././////evil.txt"), + notMatchTestInfo("/book1/:name/fixPath1/*.*", "/book1/./fixPath1/mybook/../././////evil.txt"), + notMatchTestInfo("/book2/:type:string/fixPath1/:name", "/book2/type1/fixPath1/name1/../../././////evilType/evilName"), + notMatchTestInfo("/book2/:type:string/fixPath1/:name", "/book2/type1/fixPath1/name1/../../././////evilType/evilName"), + notMatchTestInfo("/book2/:type:string/fixPath1/:name", "/book2/type1/fixPath1/name1/../../././////evilType/evilName"), } }