Repository: tenable/terrascan (language: python)

Issue:
Recursive Loop Scanning Terraform
* terrascan version: v1.16.0
* Operating System: MacOS/Docker

### Description

Terraform scan of a file that contains a module passing a parameter that uses the same name as the local that is used for the same function inside the module.

This generates a recursive loop within Terrascan.

### What I Did

I ran in debug mode and could see that it was recursively expanding the internal local variable. I isolated the error into the code in the attached zip file.

[locals_loop_error.zip](https://github.com/accurics/terrascan/files/6624667/locals_loop_error.zip)

This is similar to a previous error that was fixed.



Discussion / hints:

Fix patch:
diff --git a/pkg/iac-providers/terraform/commons/local-references.go b/pkg/iac-providers/terraform/commons/local-references.go
index b28583a04..661099efa 100644
--- a/pkg/iac-providers/terraform/commons/local-references.go
+++ b/pkg/iac-providers/terraform/commons/local-references.go
@@ -17,6 +17,7 @@
 package commons
 
 import (
+	"encoding/json"
 	"io/ioutil"
 	"reflect"
 	"regexp"
@@ -85,9 +86,23 @@ func (r *RefResolver) ResolveLocalRef(localRef, callerRef string) interface{} {
 		return localRef
 	}
 
+	valKind := reflect.TypeOf(val).Kind()
+
 	// replace the local value reference string with actual value
-	if reflect.TypeOf(val).Kind() == reflect.String {
-		valStr := val.(string)
+	if valKind == reflect.String || valKind == reflect.Map {
+		valStr := ""
+
+		if valKind == reflect.Map {
+			data, err := json.Marshal(val)
+			if err != nil {
+				zap.S().Errorf("failed to convert expression '%v', ref: '%v'", localAttr.Expr, localRef)
+				return localRef
+			}
+			valStr = string(data)
+		} else {
+			valStr = val.(string)
+		}
+
 		resolvedVal := strings.Replace(localRef, localExpr, valStr, 1)
 		if callerRef != "" && strings.Contains(resolvedVal, callerRef) {
 			zap.S().Debugf("resolved str local value ref: '%v', value: '%v'", localRef, resolvedVal)
diff --git a/pkg/iac-providers/terraform/commons/references.go b/pkg/iac-providers/terraform/commons/references.go
index da6dfc294..b5e525824 100644
--- a/pkg/iac-providers/terraform/commons/references.go
+++ b/pkg/iac-providers/terraform/commons/references.go
@@ -17,10 +17,12 @@
 package commons
 
 import (
+	"encoding/json"
 	"reflect"
 	"regexp"
 
 	hclConfigs "github.com/hashicorp/terraform/configs"
+	"go.uber.org/zap"
 )
 
 var (
@@ -67,8 +69,20 @@ func (r *RefResolver) ResolveRefs(config jsonObj) jsonObj {
 
 			// case 1: config value is a string; in resource config, refs
 			// are of the type string
-			config[k] = r.ResolveStrRef(v.(string), "")
+			value := r.ResolveStrRef(v.(string), "")
+
+			// check if value is string and can be converted to map
+			if valueStr, ok := value.(string); ok {
+				var temp jsonObj
+				err := json.Unmarshal([]byte(valueStr), &temp)
+				if err != nil {
+					zap.S().Debugf("failed to unmarshal json string %s", valueStr)
+				} else {
+					value = temp
+				}
+			}
 
+			config[k] = value
 		case vType == "tfv12.jsonObj" && vKind == reflect.Map:
 
 			// case 2: config value is of type jsonObj
diff --git a/pkg/iac-providers/terraform/commons/variable-references.go b/pkg/iac-providers/terraform/commons/variable-references.go
index 4d4b066f5..43cb9f6ea 100644
--- a/pkg/iac-providers/terraform/commons/variable-references.go
+++ b/pkg/iac-providers/terraform/commons/variable-references.go
@@ -17,6 +17,7 @@
 package commons
 
 import (
+	"encoding/json"
 	"io/ioutil"
 	"reflect"
 	"regexp"
@@ -87,8 +88,22 @@ func (r *RefResolver) ResolveVarRef(varRef, callerRef string) interface{} {
 	}
 	zap.S().Debugf("resolved variable ref '%v', value: '%v'", varRef, val)
 
-	if reflect.TypeOf(val).Kind() == reflect.String {
-		valStr := val.(string)
+	valKind := reflect.TypeOf(val).Kind()
+
+	if valKind == reflect.String || valKind == reflect.Map {
+		valStr := ""
+
+		if valKind == reflect.Map {
+			data, err := json.Marshal(val)
+			if err != nil {
+				zap.S().Errorf("failed to convert expression '%v', ref: '%v'", hclVar, varRef)
+				return varRef
+			}
+			valStr = string(data)
+		} else {
+			valStr = val.(string)
+		}
+
 		resolvedVal := strings.Replace(varRef, varExpr, valStr, 1)
 		if varRef == resolvedVal {
 			zap.S().Debugf("resolved str variable ref refers to self: '%v'", varRef)
@@ -150,6 +165,16 @@ func (r *RefResolver) ResolveVarRefFromParentModuleCall(varRef, callerRef string
 	// replace the variable reference string with actual value
 	if reflect.TypeOf(val).Kind() == reflect.String {
 		valStr := val.(string)
+
+		// if resolved variable value from parent module is local reference get the local value from parent module
+		if isLocalRef(valStr) {
+			t := NewRefResolver(r.Config.Parent, nil)
+			localVal := t.ResolveLocalRef(valStr, varRef)
+			if reflect.TypeOf(localVal).Kind() == reflect.String {
+				valStr = localVal.(string)
+			}
+		}
+
 		resolvedVal := strings.Replace(varRef, varExpr, valStr, 1)
 		if strings.Contains(valStr, varExpr) {
 			zap.S().Debugf("resolved str variable ref refers to self: '%v'", varRef)


Test patch:
diff --git a/pkg/iac-providers/terraform/v12/testdata/tfjson/moduleconfigs.json b/pkg/iac-providers/terraform/v12/testdata/tfjson/moduleconfigs.json
index e0dfb4336..ef2b4d73f 100644
--- a/pkg/iac-providers/terraform/v12/testdata/tfjson/moduleconfigs.json
+++ b/pkg/iac-providers/terraform/v12/testdata/tfjson/moduleconfigs.json
@@ -403,7 +403,18 @@
         "kms_data_key_reuse_period_seconds": 300,
         "kms_master_key_id": "alias/aws/sqs",
         "name": "terraform-example-queue",
-        "policy": "{\n  \"Version\": \"2012-10-17\",\n  \"Statement\": [{\n    \"Sid\":\"Queue1_AnonymousAccess_AllActions_WhitelistIP\",\n    \"Effect\": \"Allow\",\n    \"Principal\": \"*\",\n    \"Action\": \"sqs:*\",\n    \"Resource\": \"arn:aws:sqs:*:111122223333:queue1\"\n  }] \n}\n"
+        "policy": {
+          "Statement": [
+            {
+              "Action": "sqs:*",
+              "Effect": "Allow",
+              "Principal": "*",
+              "Resource": "arn:aws:sqs:*:111122223333:queue1",
+              "Sid": "Queue1_AnonymousAccess_AllActions_WhitelistIP"
+            }
+          ],
+          "Version": "2012-10-17"
+        }
       },
       "skip_rules": null,
       "max_severity": "",
diff --git a/pkg/iac-providers/terraform/v14/load-dir_test.go b/pkg/iac-providers/terraform/v14/load-dir_test.go
index f9aecd525..aa9bb7c70 100644
--- a/pkg/iac-providers/terraform/v14/load-dir_test.go
+++ b/pkg/iac-providers/terraform/v14/load-dir_test.go
@@ -241,6 +241,14 @@ func TestLoadIacDir(t *testing.T) {
 			nonRecursive: true,
 			wantErr:      nilMultiErr,
 		},
+		{
+			name:         "recursive loop while resolving locals with same name in parent and child module",
+			tfConfigDir:  filepath.Join(testDataDir, "recursive-loop-duplicate-locals"),
+			tfJSONFile:   filepath.Join(tfJSONDir, "recursive-loop-duplicate-locals.json"),
+			tfv14:        TfV14{},
+			nonRecursive: true,
+			wantErr:      nilMultiErr,
+		},
 	}
 
 	for _, tt := range table2 {
diff --git a/pkg/iac-providers/terraform/v14/testdata/recursive-loop-duplicate-locals/dummy/main.tf b/pkg/iac-providers/terraform/v14/testdata/recursive-loop-duplicate-locals/dummy/main.tf
new file mode 100644
index 000000000..6de09bf7d
--- /dev/null
+++ b/pkg/iac-providers/terraform/v14/testdata/recursive-loop-duplicate-locals/dummy/main.tf
@@ -0,0 +1,21 @@
+variable "tags" {
+  type = map
+}
+
+variable "name" { type = string }
+variable "prefix" { type = string }
+
+locals {
+  common_tags = merge({
+          CreatedBy = "Terraform"
+        }, var.tags)
+  name = var.prefix != "" ? "${var.prefix}-${var.name}" : var.name
+}
+
+resource "aws_iam_user" "lb" {
+  name = local.name
+  tags = merge(local.common_tags,
+  {
+    Name = local.name
+  })
+}
diff --git a/pkg/iac-providers/terraform/v14/testdata/recursive-loop-duplicate-locals/main.tf b/pkg/iac-providers/terraform/v14/testdata/recursive-loop-duplicate-locals/main.tf
new file mode 100644
index 000000000..5c18a5f2e
--- /dev/null
+++ b/pkg/iac-providers/terraform/v14/testdata/recursive-loop-duplicate-locals/main.tf
@@ -0,0 +1,16 @@
+locals {
+  common_tags = {
+    this = "that"
+  }
+}
+
+module "dummy" {
+  source = "./dummy"
+  
+  tags = local.common_tags  
+  name   = "fred"
+  prefix = "joe"
+}
+
+
+
diff --git a/pkg/iac-providers/terraform/v14/testdata/tfjson/moduleconfigs.json b/pkg/iac-providers/terraform/v14/testdata/tfjson/moduleconfigs.json
index 4742a97d9..8f960b1c2 100644
--- a/pkg/iac-providers/terraform/v14/testdata/tfjson/moduleconfigs.json
+++ b/pkg/iac-providers/terraform/v14/testdata/tfjson/moduleconfigs.json
@@ -403,12 +403,22 @@
         "kms_data_key_reuse_period_seconds": 300,
         "kms_master_key_id": "alias/aws/sqs",
         "name": "terraform-example-queue",
-        "policy": "{\n  \"Version\": \"2012-10-17\",\n  \"Statement\": [{\n    \"Sid\":\"Queue1_AnonymousAccess_AllActions_WhitelistIP\",\n    \"Effect\": \"Allow\",\n    \"Principal\": \"*\",\n    \"Action\": \"sqs:*\",\n    \"Resource\": \"arn:aws:sqs:*:111122223333:queue1\"\n  }] \n}\n"
+        "policy": {
+          "Statement": [
+            {
+              "Action": "sqs:*",
+              "Effect": "Allow",
+              "Principal": "*",
+              "Resource": "arn:aws:sqs:*:111122223333:queue1",
+              "Sid": "Queue1_AnonymousAccess_AllActions_WhitelistIP"
+            }
+          ],
+          "Version": "2012-10-17"
+        }
       },
       "skip_rules": null,
       "max_severity": "",
       "min_severity": ""
-      
     }
   ]
-}
\ No newline at end of file
+}
diff --git a/pkg/iac-providers/terraform/v14/testdata/tfjson/recursive-loop-duplicate-locals.json b/pkg/iac-providers/terraform/v14/testdata/tfjson/recursive-loop-duplicate-locals.json
new file mode 100644
index 000000000..d14d8aaec
--- /dev/null
+++ b/pkg/iac-providers/terraform/v14/testdata/tfjson/recursive-loop-duplicate-locals.json
@@ -0,0 +1,20 @@
+{
+    "aws_iam_user": [
+      {
+        "id": "aws_iam_user.lb",
+        "name": "lb",
+        "module_name": "dummy",
+        "source": "dummy/main.tf",
+        "plan_root": "./",
+        "line": 15,
+        "type": "aws_iam_user",
+        "config": {
+          "name": "joe != \"\" ? \"joe-fred\" : fred",
+          "tags": "${merge(${merge({\n          CreatedBy = \"Terraform\"\n        }, {\"this\":\"that\"})},\n  {\n    Name = joe != \"\" ? \"joe-fred\" : fred\n  })}"
+        },
+        "skip_rules": null,
+        "max_severity": "",
+        "min_severity": ""
+      }
+    ]
+  }
\ No newline at end of file
diff --git a/pkg/iac-providers/terraform/v14/testdata/tfjson/recursive-loop-locals.json b/pkg/iac-providers/terraform/v14/testdata/tfjson/recursive-loop-locals.json
index 19338345f..de3989073 100644
--- a/pkg/iac-providers/terraform/v14/testdata/tfjson/recursive-loop-locals.json
+++ b/pkg/iac-providers/terraform/v14/testdata/tfjson/recursive-loop-locals.json
@@ -9,7 +9,7 @@
       "line": 13,
       "type": "aws_iam_user",
       "config": {
-        "name": "${lower(${local.foo} != null ? var.bar : var.foo)}"
+        "name": "${lower(bar != null ? bar : bar)}"
       },
       "skip_rules": null
     }