[v11.0/forgejo] fix: prevent .forgejo/template from being out-of-repo content

Author: mfenniak

services/repository/generate.go

@@ -9,7 +9,6 @@ import (
 	"context"
 	"fmt"
 	"os"
-	"path/filepath"
 	"regexp"
 	"strconv"
 	"strings"
@@ -124,35 +123,6 @@ func (gt *GiteaTemplate) Globs() []glob.Glob {
 	return gt.globs
 }
 
-func checkGiteaTemplate(tmpDir string) (*GiteaTemplate, error) {
-	configDirs := []string{".forgejo", ".gitea"}
-	var templateFilePath string
-
-	for _, dir := range configDirs {
-		candidatePath := filepath.Join(tmpDir, dir, "template")
-		if _, err := os.Stat(candidatePath); err == nil {
-			templateFilePath = candidatePath
-			break
-		} else if !os.IsNotExist(err) {
-			return nil, err
-		}
-	}
-
-	if templateFilePath == "" {
-		return nil, nil
-	}
-
-	content, err := os.ReadFile(templateFilePath)
-	if err != nil {
-		return nil, err
-	}
-
-	return &GiteaTemplate{
-		Path:    templateFilePath,
-		Content: content,
-	}, nil
-}
-
 func generateGitContent(ctx context.Context, repo, templateRepo, generateRepo *repo_model.Repository) (err error) {
 	tmpDir, err := os.MkdirTemp(os.TempDir(), "gitea-"+repo.Name)
 	if err != nil {

services/repository/generate_repo_commit.go

@@ -140,3 +140,39 @@ func generateRepoCommit(ctx context.Context, repo, templateRepo, generateRepo *r
 
 	return initRepoCommit(ctx, tmpDir, repo, repo.Owner, defaultBranch)
 }
+
+func checkGiteaTemplate(tmpDir string) (*GiteaTemplate, error) {
+	configDirs := []string{".forgejo", ".gitea"}
+	var templateFilePath string
+
+	// All file access should be done through `root` to avoid file traversal attacks, especially with symlinks
+	root, err := os.OpenRoot(tmpDir)
+	if err != nil {
+		return nil, fmt.Errorf("open root: %w", err)
+	}
+	defer root.Close()
+
+	for _, dir := range configDirs {
+		candidatePath := filepath.Join(dir, "template")
+		if _, err := root.Stat(candidatePath); err == nil {
+			templateFilePath = candidatePath
+			break
+		} else if !os.IsNotExist(err) {
+			return nil, err
+		}
+	}
+
+	if templateFilePath == "" {
+		return nil, nil
+	}
+
+	content, err := root.ReadFile(templateFilePath)
+	if err != nil {
+		return nil, err
+	}
+
+	return &GiteaTemplate{
+		Path:    templateFilePath,
+		Content: content,
+	}, nil
+}

services/repository/generate_repo_commit_legacy.go

@@ -163,3 +163,44 @@ func generateRepoCommit(ctx context.Context, repo, templateRepo, generateRepo *r
 
 	return initRepoCommit(ctx, tmpDir, repo, repo.Owner, defaultBranch)
 }
+
+func checkGiteaTemplate(tmpDir string) (*GiteaTemplate, error) {
+	configDirs := []string{".forgejo", ".gitea"}
+	var templateFilePath string
+
+	// All file access should be done through `root` to avoid file traversal attacks, especially with symlinks
+	root, err := os.OpenRoot(tmpDir)
+	if err != nil {
+		return nil, fmt.Errorf("open root: %w", err)
+	}
+	defer root.Close()
+
+	for _, dir := range configDirs {
+		candidatePath := filepath.Join(dir, "template")
+		if _, err := root.Stat(candidatePath); err == nil {
+			templateFilePath = candidatePath
+			break
+		} else if !os.IsNotExist(err) {
+			return nil, err
+		}
+	}
+
+	if templateFilePath == "" {
+		return nil, nil
+	}
+
+	// FIXME: root.ReadFile(relPath) in go 1.25
+	file, err := root.Open(templateFilePath)
+	if err != nil {
+		return nil, err
+	}
+	content, err := io.ReadAll(file)
+	if err != nil {
+		return nil, err
+	}
+
+	return &GiteaTemplate{
+		Path:    templateFilePath,
+		Content: content,
+	}, nil
+}

tests/integration/repo_generate_test.go

@@ -344,3 +344,43 @@ func TestRepoGenerateTemplatingSymlink(t *testing.T) {
 		}
 	})
 }
+
+func TestRepoGenerateTemplatingSymlinkGlobFile(t *testing.T) {
+	onGiteaRun(t, func(t *testing.T, u *url.URL) {
+		templateName := "my_template"
+		generatedName := "my_generated"
+
+		userName := "user1"
+		session := loginUser(t, userName)
+		user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: userName})
+
+		template, _, f := tests.CreateDeclarativeRepoWithOptions(t, user, tests.DeclarativeRepoOptions{
+			Name:       optional.Some(templateName),
+			IsTemplate: optional.Some(true),
+			Files: optional.Some([]*files_service.ChangeRepoFile{
+				{
+					Operation:     "create",
+					TreePath:      ".forgejo/template",
+					ContentReader: strings.NewReader("/etc/passwd"),
+					Symlink:       true,
+				},
+			}),
+		})
+		defer f()
+
+		// The repo.TemplateID field is not initialized. Luckily, the ID field holds the expected value
+		templateID := strconv.FormatInt(template.ID, 10)
+
+		resp := testRepoGenerateFailure(
+			t,
+			session,
+			templateID,
+			user.Name,
+			templateName,
+			user,
+			user,
+			generatedName,
+		)
+		assert.Contains(t, resp.Body.String(), "statat .forgejo/template: path escapes from parent")
+	})
+}