add extra validation checks to isDelegation

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

tuf/data/roles.go

@@ -3,6 +3,8 @@ package data
 import (
 	"fmt"
 	"strings"
+	"regexp"
+	"path/filepath"
 )
 
 // Canonical base role names
@@ -114,7 +116,9 @@ func ValidRole(name string) bool {
 // IsDelegation checks if the role is a delegation or a root role
 func IsDelegation(role string) bool {
 	targetsBase := fmt.Sprintf("%s/", ValidRoles[CanonicalTargetsRole])
-	return strings.HasPrefix(role, targetsBase) && !strings.HasSuffix(role, "/")
+	whitelistedChars, _ := regexp.MatchString("^[a-zA-Z0-9_/]*$", role)
+	isClean := filepath.Clean(role) == role
+	return strings.HasPrefix(role, targetsBase) && !strings.HasSuffix(role, "/") && whitelistedChars && isClean
 }
 
 // RootRole is a cut down role as it appears in the root.json

tuf/data/roles_test.go

@@ -195,6 +195,27 @@ func TestIsDelegation(t *testing.T) {
 	assert.False(t, IsDelegation(CanonicalTargetsRole))
 	assert.False(t, IsDelegation(CanonicalTargetsRole+"/"))
 	assert.False(t, IsDelegation(filepath.Join(CanonicalTargetsRole, "level1")+"/"))
+
+	assert.False(t, IsDelegation(
+		filepath.Join(CanonicalTargetsRole, "directory") + "/../../traversal"))
+
+	assert.False(t, IsDelegation(
+		filepath.Join(CanonicalTargetsRole) + "///test/middle/slashes"))
+
+	assert.False(t, IsDelegation(
+		filepath.Join(CanonicalTargetsRole) + "/./././"))
+
+	assert.False(t, IsDelegation(
+		filepath.Join("  ", CanonicalTargetsRole, "level1")))
+
+	assert.False(t, IsDelegation(
+		filepath.Join("  " + CanonicalTargetsRole, "level1")))
+
+	assert.False(t, IsDelegation(
+		filepath.Join(CanonicalTargetsRole, "level1" + "  ")))
+
+	assert.False(t, IsDelegation(
+		filepath.Join(CanonicalTargetsRole, "white   space" + "level2")))
 }
 
 func TestValidRoleFunction(t *testing.T) {