From 5beb13b21aa2d4d62467a19a92dfbe3f223bfe03 Mon Sep 17 00:00:00 2001
From: Juliusz Chroboczek <jch@irif.fr>
Date: Tue, 24 Aug 2021 00:31:46 +0200
Subject: [PATCH] Early paranoia in group name validation.

We will fail malicious paths in openDescriptionFile, but it
doesn't harm to be paranoid early.
---
 group/group.go      | 16 +++++++++++++++-
 group/group_test.go | 29 +++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/group/group.go b/group/group.go
index 6f763d6..f4b1702 100644
--- a/group/group.go
+++ b/group/group.go
@@ -373,8 +373,22 @@ func Add(name string, desc *Description) (*Group, error) {
 	return g, err
 }
 
+func validGroupName(name string) bool {
+	if filepath.Separator != '/' &&
+		strings.ContainsRune(name, filepath.Separator) {
+		return false
+	}
+
+	s := path.Clean("/" + name)
+	if s == "/" {
+		return false
+	}
+
+	return s == "/"+name
+}
+
 func add(name string, desc *Description) (*Group, []Client, error) {
-	if name == "" || strings.HasSuffix(name, "/") {
+	if !validGroupName(name) {
 		return nil, nil, UserError("illegal group name")
 	}
 
diff --git a/group/group_test.go b/group/group_test.go
index 664a263..dd2dc1d 100644
--- a/group/group_test.go
+++ b/group/group_test.go
@@ -245,3 +245,32 @@ func TestFmtpValue(t *testing.T) {
 		}
 	}
 }
+
+func TestValidGroupName(t *testing.T) {
+	type nameTest struct {
+		name   string
+		result bool
+	}
+	tests := []nameTest{
+		{"", false},
+		{"/", false},
+		{"/foo", false},
+		{"foo/", false},
+		{"./foo", false},
+		{"foo/.", false},
+		{"../foo", false},
+		{"foo/..", false},
+		{"foo/./bar", false},
+		{"foo/../bar", false},
+		{"foo", true},
+		{"foo/bar", true},
+	}
+
+	for _, test := range tests {
+		r := validGroupName(test.name)
+		if r != test.result {
+			t.Errorf("Valid %v: got %v, expected %v",
+				test.name, r, test.result)
+		}
+	}
+}