From 1ad48f781eb0681561b083b49dfeff84ba51f2fe Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 28 Mar 2024 00:55:05 +0800 Subject: [PATCH] Relax generic package filename restrictions (#30135) Now, the chars `=:;()[]{}~!@#$%^ &` are possible as well Fixes #30134 --------- Co-authored-by: KN4CK3R --- routers/api/packages/generic/generic.go | 29 +++++++-- routers/api/packages/generic/generic_test.go | 65 +++++++++++++++++++ .../integration/api_packages_generic_test.go | 4 +- 3 files changed, 91 insertions(+), 7 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 b65870a8d..823293113 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 = packageNameRegex + packageNameRegex = regexp.MustCompile(`\A[-_+.\w]+\z`) + filenameRegex = regexp.MustCompile(`\A[-_+=:;.()\[\]{}~!@#$%^& \w]+\z`) ) func apiError(ctx *context.Context, status int, obj any) { @@ -54,20 +55,38 @@ 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])) && !unicode.IsNumber(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) { + apiError(ctx, http.StatusBadRequest, errors.New("invalid package name")) + return + } + + if !isValidFileName(filename) { + apiError(ctx, http.StatusBadRequest, errors.New("invalid 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 000000000..1acaafe57 --- /dev/null +++ b/routers/api/packages/generic/generic_test.go @@ -0,0 +1,65 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +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", + "1", + "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", + "1", + "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 93525ac4b..1cbae599a 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) })