fix: remove billion-laughs attack vector

The previous implementation was vulnerable to a billion-laughs attack,
where someone could interpolate values based upon other values,
something like:

```yaml

values:
  lol1: lol
  lol2: '{{values.lol1}}{{values.lol1}}' #
  lol3: '{{values.lol2}}{{values.lol2}}{{values.lol2}}{{values.lol2}}'
```

To counteract that, instead of directly manipulating the `params` map,
we create a map to keep track of the interpolated values, and only
template the values which have been previously whitelisted. Once we go
through all the values, we then merge the interpolated values map back
to the `params` map.

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

applicationset/generators/cluster.go

@@ -109,23 +109,21 @@ func (g *ClusterGenerator) GenerateParams(
 			params["name"] = cluster.Name
 			params["server"] = cluster.Server
 
-			for key, value := range appSetGenerator.Clusters.Values {
-				// NB: This will only template name and server since those are the only values we will have here. Is that desirable?
-				err := replaceTemplatedString(value, params, key)
-				if err != nil {
-					return nil, err
-				}
+			err = appendTemplatedValues(appSetGenerator.Clusters.Values, params)
+			if err != nil {
+				return nil, err
 			}
 
-			log.WithField("cluster", "local cluster").Info("matched local cluster")
-
 			res = append(res, params)
+
+			log.WithField("cluster", "local cluster").Info("matched local cluster")
 		}
 	}
 
 	// For each matching cluster secret (non-local clusters only)
 	for _, cluster := range secretsFound {
 		params := map[string]string{}
+
 		params["name"] = string(cluster.Data["name"])
 		params["nameNormalized"] = sanitizeName(string(cluster.Data["name"]))
 		params["server"] = string(cluster.Data["server"])
@@ -135,30 +133,50 @@ func (g *ClusterGenerator) GenerateParams(
 		for key, value := range cluster.ObjectMeta.Labels {
 			params[fmt.Sprintf("metadata.labels.%s", key)] = value
 		}
-		for key, value := range appSetGenerator.Clusters.Values {
-			// TODO: Do we want to allow interpolation from other values? If so we'd need to do a second pass. For now
-			// only interpolations from labels, annotations, server and name should be supported.
-			err := replaceTemplatedString(value, params, key)
-			if err != nil {
-				return nil, err
-			}
+
+		err = appendTemplatedValues(appSetGenerator.Clusters.Values, params)
+		if err != nil {
+			return nil, err
 		}
-		log.WithField("cluster", cluster.Name).Info("matched cluster secret")
 
 		res = append(res, params)
+
+		log.WithField("cluster", cluster.Name).Info("matched cluster secret")
 	}
 
 	return res, nil
 }
 
-func replaceTemplatedString(value string, params map[string]string, key string) error {
+func appendTemplatedValues(clusterValues map[string]string, params map[string]string) error {
+	// We create a local map to ensure that we do not fall victim to a billion-laughs attack. We iterate through the
+	// cluster values map and only replace values in said map if it has already been whitelisted in the params map.
+	// Once we iterate through all the cluster values we can then safely merge the `tmp` map into the main params map.
+	tmp := map[string]string{}
+
+	for key, value := range clusterValues {
+		result, err := replaceTemplatedString(value, params)
+
+		if err != nil {
+			return err
+		}
+
+		tmp[fmt.Sprintf("values.%s", key)] = result
+	}
+
+	for key, value := range tmp {
+		params[key] = value
+	}
+
+	return nil
+}
+
+func replaceTemplatedString(value string, params map[string]string) (string, error) {
 	fstTmpl := fasttemplate.New(value, "{{", "}}")
 	replacedTmplStr, err := render.Replace(fstTmpl, params, true)
 	if err != nil {
-		return err
+		return "", err
 	}
-	params[fmt.Sprintf("values.%s", key)] = replacedTmplStr
-	return nil
+	return replacedTmplStr, nil
 }
 
 func (g *ClusterGenerator) getSecretsByClusterName(appSetGenerator *argoappsetv1alpha1.ApplicationSetGenerator) (map[string]corev1.Secret, error) {

applicationset/generators/cluster_test.go

@@ -95,19 +95,22 @@ func TestGenerateParams(t *testing.T) {
 			name:     "no label selector",
 			selector: metav1.LabelSelector{},
 			values: map[string]string{
+				"lol1":  "lol",
+				"lol2":  "{{values.lol1}}{{values.lol1}}",
+				"lol3":  "{{values.lol2}}{{values.lol2}}{{values.lol2}}",
 				"foo":   "bar",
 				"bar":   "{{ metadata.annotations.foo.argoproj.io }}",
 				"bat":   "{{ metadata.labels.environment }}",
 				"aaa":   "{{ server }}",
 				"no-op": "{{ this-does-not-exist }}",
 			}, expected: []map[string]string{
-				{"values.foo": "bar", "values.bar": "production", "values.no-op": "{{ this-does-not-exist }}", "values.bat": "production", "values.aaa": "https://production-01.example.com", "name": "production_01/west", "nameNormalized": "production-01-west", "server": "https://production-01.example.com", "metadata.labels.environment": "production", "metadata.labels.org": "bar",
+				{"values.lol1": "lol", "values.lol2": "{{values.lol1}}{{values.lol1}}", "values.lol3": "{{values.lol2}}{{values.lol2}}{{values.lol2}}", "values.foo": "bar", "values.bar": "production", "values.no-op": "{{ this-does-not-exist }}", "values.bat": "production", "values.aaa": "https://production-01.example.com", "name": "production_01/west", "nameNormalized": "production-01-west", "server": "https://production-01.example.com", "metadata.labels.environment": "production", "metadata.labels.org": "bar",
 					"metadata.labels.argocd.argoproj.io/secret-type": "cluster", "metadata.annotations.foo.argoproj.io": "production"},
 
-				{"values.foo": "bar", "values.bar": "staging", "values.no-op": "{{ this-does-not-exist }}", "values.bat": "staging", "values.aaa": "https://staging-01.example.com", "name": "staging-01", "nameNormalized": "staging-01", "server": "https://staging-01.example.com", "metadata.labels.environment": "staging", "metadata.labels.org": "foo",
+				{"values.lol1": "lol", "values.lol2": "{{values.lol1}}{{values.lol1}}", "values.lol3": "{{values.lol2}}{{values.lol2}}{{values.lol2}}", "values.foo": "bar", "values.bar": "staging", "values.no-op": "{{ this-does-not-exist }}", "values.bat": "staging", "values.aaa": "https://staging-01.example.com", "name": "staging-01", "nameNormalized": "staging-01", "server": "https://staging-01.example.com", "metadata.labels.environment": "staging", "metadata.labels.org": "foo",
 					"metadata.labels.argocd.argoproj.io/secret-type": "cluster", "metadata.annotations.foo.argoproj.io": "staging"},
 
-				{"values.foo": "bar", "values.bar": "{{ metadata.annotations.foo.argoproj.io }}", "values.no-op": "{{ this-does-not-exist }}", "values.bat": "{{ metadata.labels.environment }}", "values.aaa": "https://kubernetes.default.svc", "name": "in-cluster", "server": "https://kubernetes.default.svc"},
+				{"values.lol1": "lol", "values.lol2": "{{values.lol1}}{{values.lol1}}", "values.lol3": "{{values.lol2}}{{values.lol2}}{{values.lol2}}", "values.foo": "bar", "values.bar": "{{ metadata.annotations.foo.argoproj.io }}", "values.no-op": "{{ this-does-not-exist }}", "values.bat": "{{ metadata.labels.environment }}", "values.aaa": "https://kubernetes.default.svc", "name": "in-cluster", "server": "https://kubernetes.default.svc"},
 			},
 			clientError:   false,
 			expectedError: nil,