[v15] fix: support set.add on nil sets in traits expression parser (#…

…49432)

Backport #49385 to branch/v15

This is a manual backport and a much smaller and more targeted change
than the original PR, because in this branch lib/expression.Set has not
been converted to use lib/utils.Set.

Changelog: Fixed a potential panic in login rule and SAML IdP expression parser

lib/expression/evaluator_test.go

@@ -29,15 +29,13 @@ import (
 	"github.com/gravitational/teleport/lib/utils/typical"
 )
 
-func TestEvaluateTraitsMap(t *testing.T) {
-	t.Parallel()
-
-	baseInputTraits := map[string][]string{
+var (
+	baseInputTraits = map[string][]string{
 		"groups":   []string{"devs", "security"},
 		"username": []string{"alice"},
 	}
 
-	tests := []struct {
+	testCases = []struct {
 		desc           string
 		expressions    map[string][]string
 		inputTraits    map[string][]string
@@ -253,7 +251,31 @@ func TestEvaluateTraitsMap(t *testing.T) {
 				"localEmails": {"alice", "bob", "charlie", "darrell", "esther", "frank"},
 			},
 		},
+		{
+			desc: "methods on nil set from nonexistent map key",
+			expressions: map[string][]string{
+				"a": {`user.spec.traits["a"].add("a")`},
+				"b": {`ifelse(user.spec.traits["b"].contains("b"), set("z"), set("b"))`},
+				"c": {`ifelse(user.spec.traits["c"].contains_any(set("c")), set("z"), set("c"))`},
+				"d": {`ifelse(user.spec.traits["d"].isempty(), set("d"), set("z"))`},
+				"e": {`user.spec.traits["e"].remove("e")`},
+				"f": {`user.spec.traits["f"].remove("f").add("f")`},
+			},
+			inputTraits: baseInputTraits,
+			expectedTraits: map[string][]string{
+				"a": {"a"},
+				"b": {"b"},
+				"c": {"c"},
+				"d": {"d"},
+				"e": {},
+				"f": {"f"},
+			},
+		},
 	}
+)
+
+func TestEvaluateTraitsMap(t *testing.T) {
+	t.Parallel()
 
 	type evaluationEnv struct {
 		Traits Dict
@@ -270,7 +292,7 @@ func TestEvaluateTraitsMap(t *testing.T) {
 	attributeParser, err := NewTraitsExpressionParser[evaluationEnv](typicalEnvVar)
 	require.NoError(t, err)
 
-	for _, tc := range tests {
+	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
 			result, err := EvaluateTraitsMap[evaluationEnv](
 				evaluationEnv{
@@ -292,3 +314,35 @@ func TestEvaluateTraitsMap(t *testing.T) {
 		})
 	}
 }
+
+func FuzzTraitsExpressionParser(f *testing.F) {
+	type evaluationEnv struct {
+		Traits Dict
+	}
+	parser, err := NewTraitsExpressionParser[evaluationEnv](map[string]typical.Variable{
+		"true":  true,
+		"false": false,
+		"user.spec.traits": typical.DynamicMap[evaluationEnv, Set](func(env evaluationEnv) (Dict, error) {
+			return env.Traits, nil
+		}),
+	})
+	require.NoError(f, err)
+	for _, tc := range testCases {
+		for _, expressions := range tc.expressions {
+			for _, expression := range expressions {
+				f.Add(expression)
+			}
+		}
+	}
+	f.Fuzz(func(t *testing.T, expression string) {
+		expr, err := parser.Parse(expression)
+		if err != nil {
+			// Many/most fuzzed expressions won't parse, as long as we didn't
+			// panic that's okay.
+			return
+		}
+		// If the expression parsed, try to evaluate it, errors are okay just
+		// make sure we don't panic.
+		_, _ = expr.Evaluate(evaluationEnv{DictFromStringSliceMap(baseInputTraits)})
+	})
+}

lib/expression/set.go

@@ -35,6 +35,9 @@ func NewSet(values ...string) Set {
 }
 
 func (s Set) add(values ...string) Set {
+	if len(s) == 0 {
+		return NewSet(values...)
+	}
 	out := s.clone()
 	for _, value := range values {
 		out[value] = struct{}{}