From f756fc7b2ba35aa3e2367c2667a60119942e7f56 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 27 Mar 2024 19:51:04 +0800 Subject: [PATCH 1/5] fix --- routers/api/packages/generic/generic.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/routers/api/packages/generic/generic.go b/routers/api/packages/generic/generic.go index b65870a8d098d..b75c3c8c1f8a5 100644 --- a/routers/api/packages/generic/generic.go +++ b/routers/api/packages/generic/generic.go @@ -18,8 +18,8 @@ import ( ) var ( - packageNameRegex = regexp.MustCompile(`\A[A-Za-z0-9\.\_\-\+]+\z`) - filenameRegex = packageNameRegex + packageNameRegex = regexp.MustCompile(`\A[-_+.A-Za-z0-9]+\z`) + filenameRegex = regexp.MustCompile(`\A[-_+=:;.()\[\]{}~!@#$%^& A-Za-z0-9]+\z`) ) func apiError(ctx *context.Context, status int, obj any) { From 9486f28bd534019f9fd13edfb53a2577e47c045d Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 27 Mar 2024 20:52:02 +0800 Subject: [PATCH 2/5] improve regex, fix tests --- routers/api/packages/generic/generic.go | 24 ++++++-- routers/api/packages/generic/generic_test.go | 60 +++++++++++++++++++ .../integration/api_packages_generic_test.go | 2 +- 3 files changed, 80 insertions(+), 6 deletions(-) create mode 100644 routers/api/packages/generic/generic_test.go diff --git a/routers/api/packages/generic/generic.go b/routers/api/packages/generic/generic.go index b75c3c8c1f8a5..38e7a960ec62e 100644 --- a/routers/api/packages/generic/generic.go +++ b/routers/api/packages/generic/generic.go @@ -8,6 +8,7 @@ import ( "net/http" "regexp" "strings" + "unicode" packages_model "code.gitea.io/gitea/models/packages" "code.gitea.io/gitea/modules/log" @@ -18,8 +19,8 @@ import ( ) var ( - packageNameRegex = regexp.MustCompile(`\A[-_+.A-Za-z0-9]+\z`) - filenameRegex = regexp.MustCompile(`\A[-_+=:;.()\[\]{}~!@#$%^& A-Za-z0-9]+\z`) + packageNameRegex = regexp.MustCompile(`\A[-_+.\w]+\z`) + filenameRegex = regexp.MustCompile(`\A[-_+=:;.()\[\]{}~!@#$%^& \w]+\z`) ) func apiError(ctx *context.Context, status int, obj any) { @@ -54,20 +55,33 @@ func DownloadPackageFile(ctx *context.Context) { helper.ServePackageFile(ctx, s, u, pf) } +func isValidPackageName(packageName string) bool { + if len(packageName) == 1 && !unicode.IsLetter(rune(packageName[0])) { + return false + } + return packageNameRegex.MatchString(packageName) && packageName != ".." +} + +func isValidFileName(filename string) bool { + return filenameRegex.MatchString(filename) && + strings.TrimSpace(filename) == filename && + filename != "." && filename != ".." +} + // UploadPackage uploads the specific generic package. // Duplicated packages get rejected. func UploadPackage(ctx *context.Context) { packageName := ctx.Params("packagename") filename := ctx.Params("filename") - if !packageNameRegex.MatchString(packageName) || !filenameRegex.MatchString(filename) { - apiError(ctx, http.StatusBadRequest, errors.New("Invalid package name or filename")) + if !isValidPackageName(packageName) || isValidFileName(filename) { + apiError(ctx, http.StatusBadRequest, errors.New("invalid package name or filename")) return } packageVersion := ctx.Params("packageversion") if packageVersion != strings.TrimSpace(packageVersion) { - apiError(ctx, http.StatusBadRequest, errors.New("Invalid package version")) + apiError(ctx, http.StatusBadRequest, errors.New("invalid package version")) return } diff --git a/routers/api/packages/generic/generic_test.go b/routers/api/packages/generic/generic_test.go new file mode 100644 index 0000000000000..49bbfa8588e4a --- /dev/null +++ b/routers/api/packages/generic/generic_test.go @@ -0,0 +1,60 @@ +package generic + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestValidatePackageName(t *testing.T) { + bad := []string{ + "", + ".", + "..", + "-", + "a?b", + "a b", + "a/b", + } + for _, name := range bad { + assert.False(t, isValidPackageName(name), "bad=%q", name) + } + + good := []string{ + "a", + "a-", + "a_b", + "c.d+", + } + for _, name := range good { + assert.True(t, isValidPackageName(name), "good=%q", name) + } +} + +func TestValidateFileName(t *testing.T) { + bad := []string{ + "", + ".", + "..", + "a?b", + "a/b", + " a", + "a ", + } + for _, name := range bad { + assert.False(t, isValidFileName(name), "bad=%q", name) + } + + good := []string{ + "-", + "a", + "a-", + "a_b", + "a b", + "c.d+", + `-_+=:;.()[]{}~!@#$%^& aA1`, + } + for _, name := range good { + assert.True(t, isValidFileName(name), "good=%q", name) + } +} diff --git a/tests/integration/api_packages_generic_test.go b/tests/integration/api_packages_generic_test.go index 93525ac4b19cc..58278d1245a22 100644 --- a/tests/integration/api_packages_generic_test.go +++ b/tests/integration/api_packages_generic_test.go @@ -84,7 +84,7 @@ func TestPackageGeneric(t *testing.T) { t.Run("InvalidParameter", func(t *testing.T) { defer tests.PrintCurrentTest(t)() - req := NewRequestWithBody(t, "PUT", fmt.Sprintf("/api/packages/%s/generic/%s/%s/%s", user.Name, "invalid+package name", packageVersion, filename), bytes.NewReader(content)). + req := NewRequestWithBody(t, "PUT", fmt.Sprintf("/api/packages/%s/generic/%s/%s/%s", user.Name, "invalid|package name", packageVersion, filename), bytes.NewReader(content)). AddBasicAuth(user.Name) MakeRequest(t, req, http.StatusBadRequest) From c90ad79be3359d6b1fcf4e24aa3019fd15cdac19 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 27 Mar 2024 21:00:38 +0800 Subject: [PATCH 3/5] more tests --- routers/api/packages/generic/generic.go | 2 +- routers/api/packages/generic/generic_test.go | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/routers/api/packages/generic/generic.go b/routers/api/packages/generic/generic.go index 38e7a960ec62e..4a09e74ecb164 100644 --- a/routers/api/packages/generic/generic.go +++ b/routers/api/packages/generic/generic.go @@ -56,7 +56,7 @@ func DownloadPackageFile(ctx *context.Context) { } func isValidPackageName(packageName string) bool { - if len(packageName) == 1 && !unicode.IsLetter(rune(packageName[0])) { + if len(packageName) == 1 && !unicode.IsLetter(rune(packageName[0])) && !unicode.IsNumber(rune(packageName[0])) { return false } return packageNameRegex.MatchString(packageName) && packageName != ".." diff --git a/routers/api/packages/generic/generic_test.go b/routers/api/packages/generic/generic_test.go index 49bbfa8588e4a..e180a550d908b 100644 --- a/routers/api/packages/generic/generic_test.go +++ b/routers/api/packages/generic/generic_test.go @@ -22,6 +22,7 @@ func TestValidatePackageName(t *testing.T) { good := []string{ "a", + "1", "a-", "a_b", "c.d+", @@ -48,6 +49,7 @@ func TestValidateFileName(t *testing.T) { good := []string{ "-", "a", + "1", "a-", "a_b", "a b", From d417e36ec7cf1f31152fb8675ddb36907eea5502 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 27 Mar 2024 21:15:07 +0800 Subject: [PATCH 4/5] fix lint --- routers/api/packages/generic/generic_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/routers/api/packages/generic/generic_test.go b/routers/api/packages/generic/generic_test.go index e180a550d908b..1acaafe576ae9 100644 --- a/routers/api/packages/generic/generic_test.go +++ b/routers/api/packages/generic/generic_test.go @@ -1,3 +1,6 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + package generic import ( From 20dcd3878afb7b5a44b17e3efd7ab2fed66eeca2 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 27 Mar 2024 21:50:15 +0800 Subject: [PATCH 5/5] fix test --- routers/api/packages/generic/generic.go | 9 +++++++-- tests/integration/api_packages_generic_test.go | 4 ++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/routers/api/packages/generic/generic.go b/routers/api/packages/generic/generic.go index 4a09e74ecb164..82329311340a1 100644 --- a/routers/api/packages/generic/generic.go +++ b/routers/api/packages/generic/generic.go @@ -74,8 +74,13 @@ func UploadPackage(ctx *context.Context) { packageName := ctx.Params("packagename") filename := ctx.Params("filename") - if !isValidPackageName(packageName) || isValidFileName(filename) { - apiError(ctx, http.StatusBadRequest, errors.New("invalid package name or filename")) + if !isValidPackageName(packageName) { + apiError(ctx, http.StatusBadRequest, errors.New("invalid package name")) + return + } + + if !isValidFileName(filename) { + apiError(ctx, http.StatusBadRequest, errors.New("invalid filename")) return } diff --git a/tests/integration/api_packages_generic_test.go b/tests/integration/api_packages_generic_test.go index 58278d1245a22..1cbae599af56f 100644 --- a/tests/integration/api_packages_generic_test.go +++ b/tests/integration/api_packages_generic_test.go @@ -84,7 +84,7 @@ func TestPackageGeneric(t *testing.T) { t.Run("InvalidParameter", func(t *testing.T) { defer tests.PrintCurrentTest(t)() - req := NewRequestWithBody(t, "PUT", fmt.Sprintf("/api/packages/%s/generic/%s/%s/%s", user.Name, "invalid|package name", packageVersion, filename), bytes.NewReader(content)). + req := NewRequestWithBody(t, "PUT", fmt.Sprintf("/api/packages/%s/generic/%s/%s/%s", user.Name, "invalid package name", packageVersion, filename), bytes.NewReader(content)). AddBasicAuth(user.Name) MakeRequest(t, req, http.StatusBadRequest) @@ -92,7 +92,7 @@ func TestPackageGeneric(t *testing.T) { AddBasicAuth(user.Name) MakeRequest(t, req, http.StatusBadRequest) - req = NewRequestWithBody(t, "PUT", fmt.Sprintf("/api/packages/%s/generic/%s/%s/%s", user.Name, packageName, packageVersion, "inval+id.na me"), bytes.NewReader(content)). + req = NewRequestWithBody(t, "PUT", fmt.Sprintf("/api/packages/%s/generic/%s/%s/%s", user.Name, packageName, packageVersion, "inva|id.name"), bytes.NewReader(content)). AddBasicAuth(user.Name) MakeRequest(t, req, http.StatusBadRequest) })