From ad4c50709f656c1ac46371c1069eab3e233e828e Mon Sep 17 00:00:00 2001 From: Riyaz Faizullabhoy Date: Thu, 17 Dec 2015 17:31:13 -0800 Subject: [PATCH] add additional length and lowercase checks, change regex to explicitly reject empty string, add hyphen char Signed-off-by: Riyaz Faizullabhoy --- tuf/data/roles.go | 12 ++++++++++-- tuf/data/roles_test.go | 8 ++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/tuf/data/roles.go b/tuf/data/roles.go index 0faf870be0..3137b71a6f 100644 --- a/tuf/data/roles.go +++ b/tuf/data/roles.go @@ -116,13 +116,21 @@ 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]) - whitelistedChars, err := regexp.MatchString("^[a-zA-Z0-9_/]*$", role) + + whitelistedChars, err := regexp.MatchString("^[-a-z0-9_/]+$", role) if err != nil { return false } + + // Limit size of full role string to 255 chars for db column size limit + correctLength := len(role) < 256 + // Removes ., .., extra slashes, and trailing slash isClean := filepath.Clean(role) == role - return strings.HasPrefix(role, targetsBase) && whitelistedChars && isClean + return strings.HasPrefix(role, targetsBase) && + whitelistedChars && + correctLength && + isClean } // RootRole is a cut down role as it appears in the root.json diff --git a/tuf/data/roles_test.go b/tuf/data/roles_test.go index bc3b1edd80..48ab85b89e 100644 --- a/tuf/data/roles_test.go +++ b/tuf/data/roles_test.go @@ -187,6 +187,10 @@ func TestIsDelegation(t *testing.T) { assert.True(t, IsDelegation(filepath.Join(CanonicalTargetsRole, "level1"))) assert.True(t, IsDelegation( filepath.Join(CanonicalTargetsRole, "level1", "level2", "level3"))) + assert.True(t, IsDelegation(filepath.Join(CanonicalTargetsRole, "under_score"))) + assert.True(t, IsDelegation(filepath.Join(CanonicalTargetsRole, "hyphen-hyphen"))) + assert.False(t, IsDelegation( + filepath.Join(CanonicalTargetsRole, strings.Repeat("x", 255-len(CanonicalTargetsRole))))) assert.False(t, IsDelegation("")) assert.False(t, IsDelegation(CanonicalRootRole)) @@ -195,6 +199,7 @@ 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, "UpperCase"))) assert.False(t, IsDelegation( filepath.Join(CanonicalTargetsRole, "directory")+"/../../traversal")) @@ -216,6 +221,9 @@ func TestIsDelegation(t *testing.T) { assert.False(t, IsDelegation( filepath.Join(CanonicalTargetsRole, "white space"+"level2"))) + + assert.False(t, IsDelegation( + filepath.Join(CanonicalTargetsRole, strings.Repeat("x", 256-len(CanonicalTargetsRole))))) } func TestValidRoleFunction(t *testing.T) {