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 <wxiaoguang@gmail.com>
This commit is contained in:
metsw24-max
2026-06-10 18:03:06 +00:00
committed by GitHub
co-authored by GitHub wxiaoguang
parent fa89785d33
commit 988f0ea54a
2 changed files with 21 additions and 5 deletions
+10 -5
View File
@@ -8,7 +8,6 @@ import (
"compress/gzip" "compress/gzip"
"io" "io"
"regexp" "regexp"
"strings"
"sync" "sync"
"gitea.dev/modules/util" "gitea.dev/modules/util"
@@ -26,8 +25,14 @@ var (
ErrInvalidVersion = util.NewInvalidArgumentErrorf("package version is invalid") ErrInvalidVersion = util.NewInvalidArgumentErrorf("package version is invalid")
) )
var versionMatcher = sync.OnceValue(func() *regexp.Regexp { var globalVars = sync.OnceValue(func() (ret struct {
return regexp.MustCompile(`\A[0-9]+(?:\.[0-9a-zA-Z]+)*(?:-[0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*)?\z`) 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 // Package represents a RubyGems package
@@ -175,11 +180,11 @@ func parseMetadataFile(r io.Reader) (*Package, error) {
return nil, err return nil, err
} }
if len(spec.Name) == 0 || strings.Contains(spec.Name, "/") { if !globalVars().nameMatcher.MatchString(spec.Name) {
return nil, ErrInvalidName return nil, ErrInvalidName
} }
if !versionMatcher().MatchString(spec.Version.Version) { if !globalVars().versionMatcher.MatchString(spec.Version.Version) {
return nil, ErrInvalidVersion return nil, ErrInvalidVersion
} }
@@ -32,6 +32,17 @@ version:
assert.NoError(t, err) assert.NoError(t, err)
assert.NotNil(t, rp) 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) { func TestParseMetadataFile(t *testing.T) {