From 988f0ea54ad11315fcd912b0af7b782a951b91d4 Mon Sep 17 00:00:00 2001 From: metsw24-max Date: Wed, 10 Jun 2026 23:33:06 +0530 Subject: [PATCH] fix: validate gem name in rubygems parseMetadataFile (#38061) The registry writes the stored gem name straight into its line-based compact index, both the shared `/versions` listing (one `GEMNAME versions md5` line per gem) and the per-package `info/{name}` file. The parser only rejected an empty name or one containing a slash, so a `.gem` whose gemspec `name` carries a newline was accepted and persisted as the package name, letting an authenticated uploader forge extra lines in the shared index and so spoof additional gem names, versions and checksums to clients. The name is now checked against the upstream RubyGems name pattern in the parser, which is the layer that already validates the version. --------- Co-authored-by: wxiaoguang --- modules/packages/rubygems/metadata.go | 15 ++++++++++----- modules/packages/rubygems/metadata_test.go | 11 +++++++++++ 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/modules/packages/rubygems/metadata.go b/modules/packages/rubygems/metadata.go index 8a989bbe7f0..2258034ba45 100644 --- a/modules/packages/rubygems/metadata.go +++ b/modules/packages/rubygems/metadata.go @@ -8,7 +8,6 @@ import ( "compress/gzip" "io" "regexp" - "strings" "sync" "gitea.dev/modules/util" @@ -26,8 +25,14 @@ var ( ErrInvalidVersion = util.NewInvalidArgumentErrorf("package version is invalid") ) -var versionMatcher = sync.OnceValue(func() *regexp.Regexp { - return regexp.MustCompile(`\A[0-9]+(?:\.[0-9a-zA-Z]+)*(?:-[0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*)?\z`) +var globalVars = sync.OnceValue(func() (ret struct { + nameMatcher, versionMatcher *regexp.Regexp +}, +) { + // https://github.com/rubygems/rubygems/blob/master/lib/rubygems/specification.rb (VALID_NAME_PATTERN) + ret.nameMatcher = regexp.MustCompile(`\A[\w.-]+\z`) + ret.versionMatcher = regexp.MustCompile(`\A[0-9]+(?:\.[0-9a-zA-Z]+)*(?:-[0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*)?\z`) + return ret }) // Package represents a RubyGems package @@ -175,11 +180,11 @@ func parseMetadataFile(r io.Reader) (*Package, error) { return nil, err } - if len(spec.Name) == 0 || strings.Contains(spec.Name, "/") { + if !globalVars().nameMatcher.MatchString(spec.Name) { return nil, ErrInvalidName } - if !versionMatcher().MatchString(spec.Version.Version) { + if !globalVars().versionMatcher.MatchString(spec.Version.Version) { return nil, ErrInvalidVersion } diff --git a/modules/packages/rubygems/metadata_test.go b/modules/packages/rubygems/metadata_test.go index da75edd04af..8917bd2b36b 100644 --- a/modules/packages/rubygems/metadata_test.go +++ b/modules/packages/rubygems/metadata_test.go @@ -32,6 +32,17 @@ version: assert.NoError(t, err) assert.NotNil(t, rp) }) + + t.Run("InvalidName", func(t *testing.T) { + // a name carrying a newline would be re-emitted verbatim into the + // line-based compact index, letting an upload forge extra entries + for _, quotedName := range []string{`"evil\n1.0.0"`, `"a b"`, `"a/b"`, `""`} { + content := test.CompressGzip("name: " + quotedName + "\nversion:\n version: 1\n") + rp, err := parseMetadataFile(content) + assert.ErrorIs(t, err, ErrInvalidName, "name %s should be rejected", quotedName) + assert.Nil(t, rp) + } + }) } func TestParseMetadataFile(t *testing.T) {