Browse Source

reject empty principals

pull/7472/head
chrislu 3 weeks ago
parent
commit
528d5b8730
  1. 11
      weed/s3api/policy_conversion.go
  2. 53
      weed/s3api/policy_conversion_test.go

11
weed/s3api/policy_conversion.go

@ -101,12 +101,20 @@ func convertPrincipal(principal interface{}) (*policy_engine.StringOrStringSlice
switch p := principal.(type) {
case string:
if p == "" {
return nil, fmt.Errorf("principal string cannot be empty")
}
result := policy_engine.NewStringOrStringSlice(p)
return &result, nil
case []string:
if len(p) == 0 {
return nil, nil
}
for _, s := range p {
if s == "" {
return nil, fmt.Errorf("principal string in slice cannot be empty")
}
}
result := policy_engine.NewStringOrStringSlice(p...)
return &result, nil
case []interface{}:
@ -117,6 +125,9 @@ func convertPrincipal(principal interface{}) (*policy_engine.StringOrStringSlice
if err != nil {
return nil, fmt.Errorf("failed to convert principal array item: %w", err)
}
if str == "" {
return nil, fmt.Errorf("principal string in slice cannot be empty")
}
strs = append(strs, str)
}
}

53
weed/s3api/policy_conversion_test.go

@ -438,6 +438,59 @@ func TestConvertPrincipalUnsupportedTypes(t *testing.T) {
}
}
func TestConvertPrincipalEmptyStrings(t *testing.T) {
// Test that empty string principals are rejected for security
testCases := []struct {
name string
principal interface{}
wantError string
}{
{
name: "Empty string principal",
principal: "",
wantError: "principal string cannot be empty",
},
{
name: "Empty string in array",
principal: []string{"arn:aws:iam::123456789012:user/Alice", "", "arn:aws:iam::123456789012:user/Bob"},
wantError: "principal string in slice cannot be empty",
},
{
name: "Empty string in interface array",
principal: []interface{}{"arn:aws:iam::123456789012:user/Alice", ""},
wantError: "principal string in slice cannot be empty",
},
{
name: "Empty string in AWS map",
principal: map[string]interface{}{
"AWS": "",
},
wantError: "principal string cannot be empty",
},
{
name: "Empty string in AWS map array",
principal: map[string]interface{}{
"AWS": []string{"arn:aws:iam::123456789012:user/Alice", ""},
},
wantError: "principal string in slice cannot be empty",
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
result, err := convertPrincipal(tc.principal)
if err == nil {
t.Error("Expected error for empty principal string")
} else if !strings.Contains(err.Error(), tc.wantError) {
t.Errorf("Expected error containing %q, got: %v", tc.wantError, err)
}
if result != nil {
t.Error("Expected nil result for empty principal string")
}
})
}
}
func TestConvertStatementWithUnsupportedFields(t *testing.T) {
// Test that errors are returned for unsupported fields
// These fields are critical for policy semantics and ignoring them would be a security risk

Loading…
Cancel
Save