Skip to content

Commit 1e3b260

Browse files
committed
fix(repo): allow rm for dotted module names with safe path checks
1 parent 3d80959 commit 1e3b260

File tree

2 files changed

+77
-12
lines changed

2 files changed

+77
-12
lines changed

cli/repo/rm.go

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,48 @@ package repo
22

33
import (
44
"fmt"
5+
"path/filepath"
56
"pig/internal/config"
67
"pig/internal/output"
78
"pig/internal/utils"
9+
"regexp"
810
"strings"
911
"time"
1012

1113
"github.com/sirupsen/logrus"
1214
)
1315

16+
var rmModuleNamePattern = regexp.MustCompile(`^[A-Za-z0-9._-]+$`)
17+
18+
func resolveModulePathForRm(m *Manager, module string) (string, error) {
19+
module = strings.TrimSpace(module)
20+
if module == "" {
21+
return "", fmt.Errorf("empty module name")
22+
}
23+
if strings.Contains(module, "..") || strings.Contains(module, "/") || strings.Contains(module, `\`) {
24+
return "", fmt.Errorf("unsafe module name: %s", module)
25+
}
26+
if !rmModuleNamePattern.MatchString(module) {
27+
return "", fmt.Errorf("invalid module name: %s", module)
28+
}
29+
30+
suffix := ""
31+
switch config.OSType {
32+
case config.DistroEL:
33+
suffix = ".repo"
34+
case config.DistroDEB:
35+
suffix = ".list"
36+
default:
37+
return "", fmt.Errorf("unsupported os type: %s", config.OSType)
38+
}
39+
40+
filePath := filepath.Join(m.RepoDir, module+suffix)
41+
if !isPathWithinBase(m.RepoDir, filePath) {
42+
return "", fmt.Errorf("module path escapes repo dir")
43+
}
44+
return filePath, nil
45+
}
46+
1447
// RmRepos removes repository modules and returns a structured Result
1548
// This function is used for YAML/JSON output modes
1649
// If modules is empty, it backs up all repositories
@@ -68,23 +101,14 @@ func RmRepos(modules []string, doUpdate bool) *output.Result {
68101
if module == "" {
69102
continue
70103
}
71-
if _, ok := manager.Module[module]; !ok {
72-
data.RemovedRepos = append(data.RemovedRepos, &RemovedRepoItem{
73-
Module: module,
74-
FilePath: "",
75-
Success: false,
76-
Error: fmt.Sprintf("module not found: %s", module),
77-
})
78-
continue
79-
}
80104

81-
filePath := manager.getModulePath(module)
82-
if filePath == "" {
105+
filePath, pathErr := resolveModulePathForRm(manager, module)
106+
if pathErr != nil {
83107
data.RemovedRepos = append(data.RemovedRepos, &RemovedRepoItem{
84108
Module: module,
85109
FilePath: "",
86110
Success: false,
87-
Error: "failed to determine module path",
111+
Error: fmt.Sprintf("failed to determine module path: %v", pathErr),
88112
})
89113
continue
90114
}

cli/repo/rm_path_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package repo
2+
3+
import (
4+
"testing"
5+
6+
"pig/internal/config"
7+
)
8+
9+
func TestResolveModulePathForRm_AllowsDottedModuleOnEL(t *testing.T) {
10+
oldType := config.OSType
11+
defer func() { config.OSType = oldType }()
12+
13+
config.OSType = config.DistroEL
14+
m := &Manager{RepoDir: "/etc/yum.repos.d"}
15+
16+
path, err := resolveModulePathForRm(m, "foo.bar")
17+
if err != nil {
18+
t.Fatalf("resolveModulePathForRm returned error: %v", err)
19+
}
20+
if path != "/etc/yum.repos.d/foo.bar.repo" {
21+
t.Fatalf("unexpected path: %s", path)
22+
}
23+
}
24+
25+
func TestResolveModulePathForRm_RejectsTraversal(t *testing.T) {
26+
oldType := config.OSType
27+
defer func() { config.OSType = oldType }()
28+
29+
config.OSType = config.DistroDEB
30+
m := &Manager{RepoDir: "/etc/apt/sources.list.d"}
31+
32+
if _, err := resolveModulePathForRm(m, "../evil"); err == nil {
33+
t.Fatalf("expected traversal module to be rejected")
34+
}
35+
if _, err := resolveModulePathForRm(m, `a\b`); err == nil {
36+
t.Fatalf("expected backslash module to be rejected")
37+
}
38+
if _, err := resolveModulePathForRm(m, "a/b"); err == nil {
39+
t.Fatalf("expected slash module to be rejected")
40+
}
41+
}

0 commit comments

Comments
 (0)