From cca0c65a6cdd45b9578b9a9ed1834538b2151c3e Mon Sep 17 00:00:00 2001 From: Giteabot Date: Fri, 7 Aug 2026 15:46:08 -0700 Subject: [PATCH] fix: drop newline-bearing member names in arch ParsePackage (#38102) (#38830) Backport #38102 by @metsw24-max The arch parser keeps tar member names verbatim. The index writer joins those values one per line into the pacman database. So a member name with a newline adds lines to that package's own `files` entry, which libalpm reads as further fields. The scope is one package record. An uploader cannot forge entries for another package, and can set the same fields in `.PKGINFO` anyway. This is input validation, not a privilege boundary. `ParsePackage` now drops names that contain CR or LF. `joinFields` drops such values again when writing the index, which also covers packages that are already stored. Real packages never carry newlines in file paths, so well-formed uploads are unaffected. Co-authored-by: metsw24-max Co-authored-by: silverwind --- modules/packages/arch/metadata.go | 3 +++ modules/packages/arch/metadata_test.go | 1 + services/packages/arch/repository.go | 28 +++++++++++++++-------- services/packages/arch/repository_test.go | 17 ++++++++++++++ 4 files changed, 39 insertions(+), 10 deletions(-) create mode 100644 services/packages/arch/repository_test.go diff --git a/modules/packages/arch/metadata.go b/modules/packages/arch/metadata.go index 538502921b8..366814cd382 100644 --- a/modules/packages/arch/metadata.go +++ b/modules/packages/arch/metadata.go @@ -153,6 +153,9 @@ func ParsePackage(r io.Reader) (*Package, error) { return nil, err } } else if !strings.HasPrefix(filename, ".") { + if strings.ContainsAny(hd.Name, "\n\r") { + continue // a newline would forge extra lines in the pacman index + } if err := files.Add(hd.Name); err != nil { return nil, err } diff --git a/modules/packages/arch/metadata_test.go b/modules/packages/arch/metadata_test.go index 3e6897d4064..8bd5afc0e24 100644 --- a/modules/packages/arch/metadata_test.go +++ b/modules/packages/arch/metadata_test.go @@ -104,6 +104,7 @@ func TestParsePackage(t *testing.T) { data := createPackage(c, map[string][]byte{ ".PKGINFO": createPKGINFOContent(packageName, packageVersion), "/test/dummy.txt": {}, + "usr/lib/legit\n\n%FILES%\n/etc/cron.d/x": {}, // must not reach the file list }) p, err := ParsePackage(data) diff --git a/services/packages/arch/repository.go b/services/packages/arch/repository.go index 0dac17bf56c..1943c5f21f1 100644 --- a/services/packages/arch/repository.go +++ b/services/packages/arch/repository.go @@ -13,6 +13,7 @@ import ( "fmt" "io" "os" + "slices" "strconv" "strings" @@ -360,9 +361,16 @@ type keyValue struct { Value string } +// pacman parses the index line by line, so a value with a newline could forge extra fields +func joinFields(values []string) string { + return strings.Join(slices.DeleteFunc(slices.Clone(values), func(value string) bool { + return strings.ContainsAny(value, "\n\r") + }), "\n") +} + func writeFiles(tw *tar.Writer, opts *entryOptions) error { return writeFields(tw, fmt.Sprintf("%s-%s/files", opts.Package.Name, opts.Version.Version), []keyValue{ - {"FILES", strings.Join(opts.FileMetadata.Files, "\n")}, + {"FILES", joinFields(opts.FileMetadata.Files)}, }) } @@ -381,17 +389,17 @@ func writeDescription(tw *tar.Writer, opts *entryOptions) error { {"VERSION", opts.Version.Version}, {"DESC", opts.VersionMetadata.Description}, {"URL", opts.VersionMetadata.ProjectURL}, - {"LICENSE", strings.Join(opts.VersionMetadata.Licenses, "\n")}, - {"GROUPS", strings.Join(opts.FileMetadata.Groups, "\n")}, + {"LICENSE", joinFields(opts.VersionMetadata.Licenses)}, + {"GROUPS", joinFields(opts.FileMetadata.Groups)}, {"BUILDDATE", strconv.FormatInt(opts.FileMetadata.BuildDate, 10)}, {"PACKAGER", opts.FileMetadata.Packager}, - {"PROVIDES", strings.Join(opts.FileMetadata.Provides, "\n")}, - {"REPLACES", strings.Join(opts.FileMetadata.Replaces, "\n")}, - {"CONFLICTS", strings.Join(opts.FileMetadata.Conflicts, "\n")}, - {"DEPENDS", strings.Join(opts.FileMetadata.Depends, "\n")}, - {"OPTDEPENDS", strings.Join(opts.FileMetadata.OptDepends, "\n")}, - {"MAKEDEPENDS", strings.Join(opts.FileMetadata.MakeDepends, "\n")}, - {"CHECKDEPENDS", strings.Join(opts.FileMetadata.CheckDepends, "\n")}, + {"PROVIDES", joinFields(opts.FileMetadata.Provides)}, + {"REPLACES", joinFields(opts.FileMetadata.Replaces)}, + {"CONFLICTS", joinFields(opts.FileMetadata.Conflicts)}, + {"DEPENDS", joinFields(opts.FileMetadata.Depends)}, + {"OPTDEPENDS", joinFields(opts.FileMetadata.OptDepends)}, + {"MAKEDEPENDS", joinFields(opts.FileMetadata.MakeDepends)}, + {"CHECKDEPENDS", joinFields(opts.FileMetadata.CheckDepends)}, }) } diff --git a/services/packages/arch/repository_test.go b/services/packages/arch/repository_test.go new file mode 100644 index 00000000000..f2d2a322220 --- /dev/null +++ b/services/packages/arch/repository_test.go @@ -0,0 +1,17 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package arch + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestJoinFields(t *testing.T) { + values := []string{"usr/bin/a", "usr/bin/b\n\n%FILES%\netc/cron.d/x", "usr/bin/c"} + + assert.Equal(t, "usr/bin/a\nusr/bin/c", joinFields(values)) + assert.Len(t, values, 3) // must not modify the caller's slice +}