Repository: tfsec/tfsec
Pull request #540 (2020-12-15 22:55:06)

Fixing false positives with rules related to GCP
I tried using tfsec for our GCP projects and had many false positives. This is addressing some of them:

- Adding `google_secret_manager_secret` to whitelist of GEN003. This resource will not contain secrets but the `secret_id` is likely to be named with a sensitive attribute (e.g. secret or token) since this is a secret manager after all!
- GCP011 was failing with error when the identities in the members attribute were unknowns. 
- GCP012 was creating false positive when the `service_account` was using a reference expression.


Patch:
diff --git a/internal/app/tfsec/checks/gcp011.go b/internal/app/tfsec/checks/gcp011.go
index eec9c4473..99756e8c7 100644
--- a/internal/app/tfsec/checks/gcp011.go
+++ b/internal/app/tfsec/checks/gcp011.go
@@ -97,9 +97,8 @@ func init() {
 			} else if attributes = block.GetBlock("binding").GetAttribute("members"); attributes != nil {
 				members = attributes.Value().AsValueSlice()
 			}
-
 			for _, identities := range members {
-				if identities.Type() == cty.String && strings.HasPrefix(identities.AsString(), "user:") {
+				if identities.IsKnown() && identities.Type() == cty.String && strings.HasPrefix(identities.AsString(), "user:") {
 					return []scanner.Result{
 						check.NewResult(
 							fmt.Sprintf("'%s' grants IAM to a user object. It is recommended to manage user permissions with groups.", block.FullName()),
diff --git a/internal/app/tfsec/checks/gcp012.go b/internal/app/tfsec/checks/gcp012.go
index 7a9ea1637..a1d4b106f 100644
--- a/internal/app/tfsec/checks/gcp012.go
+++ b/internal/app/tfsec/checks/gcp012.go
@@ -60,7 +60,7 @@ func init() {
 			}
 			displayBlock := block.GetBlock("node_config")
 			serviceAccount := displayBlock.GetAttribute("service_account")
-
+			
 			if serviceAccount == nil || serviceAccount.IsEmpty() {
 				if displayBlock == nil {
 					displayBlock = block
diff --git a/internal/app/tfsec/checks/gen003.go b/internal/app/tfsec/checks/gen003.go
index 48b3568c0..04a3cb1b2 100644
--- a/internal/app/tfsec/checks/gen003.go
+++ b/internal/app/tfsec/checks/gen003.go
@@ -48,6 +48,10 @@ var sensitiveWhitelist = []struct {
 		Resource:  "aws_instance",
 		Attribute: "get_password_data",
 	},
+	{
+		Resource:  "google_secret_manager_secret",
+		Attribute: "secret_id",
+	},
 }
 
 func init() {
diff --git a/internal/app/tfsec/parser/attribute.go b/internal/app/tfsec/parser/attribute.go
index 3d2f21543..c69e250ee 100644
--- a/internal/app/tfsec/parser/attribute.go
+++ b/internal/app/tfsec/parser/attribute.go
@@ -2,12 +2,13 @@ package parser
 
 import (
 	"fmt"
+	"regexp"
+	"strings"
+
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/hcl/v2/hclsyntax"
 	"github.com/zclconf/go-cty/cty"
 	"github.com/zclconf/go-cty/cty/gocty"
-	"regexp"
-	"strings"
 
 	"github.com/tfsec/tfsec/internal/app/tfsec/debug"
 )
@@ -210,6 +211,8 @@ func (attr *Attribute) IsEmpty() bool {
 	}
 	if attr.Value().IsNull() {
 		switch t := attr.hclAttribute.Expr.(type) {
+		case *hclsyntax.ScopeTraversalExpr:
+			return false
 		case *hclsyntax.ConditionalExpr:
 			return false
 		case *hclsyntax.TemplateExpr:


Test patch:
diff --git a/internal/app/tfsec/test/gcp011_test.go b/internal/app/tfsec/test/gcp011_test.go
index 7fe12bb63..5f385b737 100644
--- a/internal/app/tfsec/test/gcp011_test.go
+++ b/internal/app/tfsec/test/gcp011_test.go
@@ -82,6 +82,18 @@ data "google_iam_policy" "test-policy" {
 }`,
 			mustIncludeResultCode: checks.GoogleUserIAMGrant,
 		},
+		{
+			name: "check data.google_iam_policy with grant to interpolated values",
+			source: `
+data "google_iam_policy" "test-policy" {
+	binding {
+		members = [
+			"serviceAccount:${google_service_account.service_account.email}"]
+		]
+	}
+}`,
+			mustExcludeResultCode: checks.GoogleUserIAMGrant,
+		},
 	}
 
 	for _, test := range tests {
diff --git a/internal/app/tfsec/test/gcp012_test.go b/internal/app/tfsec/test/gcp012_test.go
index 159908572..bbd30ac5c 100644
--- a/internal/app/tfsec/test/gcp012_test.go
+++ b/internal/app/tfsec/test/gcp012_test.go
@@ -80,6 +80,17 @@ resource "google_container_node_pool" "my-np-cluster" {
 		service_account = "anything"
 	}
 }
+`,
+			mustExcludeResultCode: checks.GCPGKENodeServiceAccount,
+		},
+		{
+			name: "defines service account in container node pool using reference",
+			source: `
+resource "google_container_node_pool" "my-np-cluster" {
+	node_config {
+		service_account = google_service_account.service_account.email
+	}
+}
 `,
 			mustExcludeResultCode: checks.GCPGKENodeServiceAccount,
 		},
diff --git a/internal/app/tfsec/test/gen003_test.go b/internal/app/tfsec/test/gen003_test.go
index a940368fe..eb004536e 100644
--- a/internal/app/tfsec/test/gen003_test.go
+++ b/internal/app/tfsec/test/gen003_test.go
@@ -36,6 +36,14 @@ resource "evil_corp" "virtual_machine" {
 			source: `
 resource "aws_efs_file_system" "myfs" {
 	creation_token = "something"
+}`,
+			mustExcludeResultCode: checks.GenericSensitiveAttributes,
+		},
+		{
+			name: "avoid false positive for google_secret_manager_secret",
+			source: `
+resource "google_secret_manager_secret" "secret" {
+	secret_id = "secret"
 }`,
 			mustExcludeResultCode: checks.GenericSensitiveAttributes,
 		},