Repository: Checkmarx/kics (language: python)

Issue:
Wrong keyExpectedValue and keyActualValue in cloudwatch_s3_policy_change_alarm_missing
In query [assets/queries/terraform/aws/cloudwatch_s3_policy_change_alarm_missing/query.rego](https://github.com/Checkmarx/kics/tree/master/assets/queries/terraform/aws/cloudwatch_s3_policy_change_alarm_missing)


	

### Expected Behavior

```
"keyExpectedValue": "aws_cloudwatch_log_metric_filter should have pattern # { ($.eventSource = \"s3.amazonaws.com\")
# && (($.eventName = PutBucketAcl) || ($.eventName = PutBucketPolicy)
# || ($.eventName = PutBucketCors) || ($.eventName = PutBucketLifecycle) ||
# ($.eventName = PutBucketReplication) || ($.eventName = DeleteBucketPolicy) ||
# ($.eventName = DeleteBucketCors) || ($.eventName = DeleteBucketLifecycle) ||
# ($.eventName = DeleteBucketReplication)) } and be associated an aws_cloudwatch_metric_alarm",
"keyActualValue": "aws_cloudwatch_log_metric_filter not filtering pattern # { ($.eventSource = \"s3.amazonaws.com\")
# && (($.eventName = PutBucketAcl) || ($.eventName = PutBucketPolicy)
# || ($.eventName = PutBucketCors) || ($.eventName = PutBucketLifecycle) ||
# ($.eventName = PutBucketReplication) || ($.eventName = DeleteBucketPolicy) ||
# ($.eventName = DeleteBucketCors) || ($.eventName = DeleteBucketLifecycle) ||
# ($.eventName = DeleteBucketReplication)) } or not associated with any aws_cloudwatch_metric_alarm",
```


### Actual Behavior

```
"keyExpectedValue": "aws_cloudwatch_log_metric_filter should have pattern { $.eventName = ConsoleLogin && $.errorMessage = \"Failed authentication\" } and be associated an aws_cloudwatch_metric_alarm",
"keyActualValue": "aws_cloudwatch_log_metric_filter not filtering pattern { $.eventName = ConsoleLogin && $.errorMessage = \"Failed authentication\" } or not associated with any aws_cloudwatch_metric_alarm",
```

This looks like a copy paste from [cloudwatch_management_console_auth_failed_alarm_missing](https://github.com/Checkmarx/kics/blob/dde42a96999fe3dd9ef4f603a05113785cf110dc/assets/queries/terraform/aws/cloudwatch_management_console_auth_failed_alarm_missing/query.rego) and does not match what this query checks.



### Specifications

-   Version: 1.7.1
-   Platform: Terraform/aws


Discussion / hints:

Fix patch:
diff --git a/assets/libraries/common.rego b/assets/libraries/common.rego
index f3dd41eef06..3f0b41de4d1 100644
--- a/assets/libraries/common.rego
+++ b/assets/libraries/common.rego
@@ -326,13 +326,13 @@ check_selector(filter, value, op, name) {
 }
 
 find_selector_by_value(filter, str) = rtn {
-	[fpath, fvalue] := walk(filter)
+	[_, fvalue] := walk(filter)
 	trim(fvalue._value, "\"") == str
 	rtn := fvalue
 } else {
-	[fpath, fvalue] := walk(filter)
+	[_, fvalue] := walk(filter)
 	trim(fvalue._value, "'") == str
-	rtn := fvalue
+	rtn = fvalue
 }
 
 get_tag_name_if_exists(resource) = name {
diff --git a/assets/queries/terraform/aws/cloudwatch_s3_policy_change_alarm_missing/query.rego b/assets/queries/terraform/aws/cloudwatch_s3_policy_change_alarm_missing/query.rego
index ea300c88036..3ab90ee52f0 100644
--- a/assets/queries/terraform/aws/cloudwatch_s3_policy_change_alarm_missing/query.rego
+++ b/assets/queries/terraform/aws/cloudwatch_s3_policy_change_alarm_missing/query.rego
@@ -1,6 +1,6 @@
 package Cx
 
-import data.generic.common as commonLib
+import data.generic.common as common_lib
 
 expressionArr := [
 	{
@@ -55,36 +55,56 @@ expressionArr := [
 	},
 ]
 
-# { ($.eventSource = \"s3.amazonaws.com\")
-# && (($.eventName = PutBucketAcl) || ($.eventName = PutBucketPolicy)
-# || ($.eventName = PutBucketCors) || ($.eventName = PutBucketLifecycle) ||
-# ($.eventName = PutBucketReplication) || ($.eventName = DeleteBucketPolicy) ||
-# ($.eventName = DeleteBucketCors) || ($.eventName = DeleteBucketLifecycle) ||
-# ($.eventName = DeleteBucketReplication)) }
-check_expression_missing(resName, filter, doc) {
-	alarm := doc.resource.aws_cloudwatch_metric_alarm[name]
-	contains(alarm.metric_name, resName)
+#{ ($.eventSource = \"s3.amazonaws.com\") && (($.eventName = PutBucketAcl) || ($.eventName = PutBucketPolicy) || ($.eventName = PutBucketCors) || ($.eventName = PutBucketLifecycle) || ($.eventName = PutBucketReplication) || ($.eventName = DeleteBucketPolicy) || ($.eventName = DeleteBucketCors) || ($.eventName = DeleteBucketLifecycle) || ($.eventName = DeleteBucketReplication)) }
+CxPolicy[result] {
+	doc := input.document[i]
+	_ := doc.resource.aws_cloudwatch_log_metric_filter[name]
+
+	
+	count([alarm | alarm := doc.resource.aws_cloudwatch_metric_alarm[_]; contains(alarm.metric_name, name)]) == 0 
+	
+	result := {
+		"documentId": input.document[i].id,
+		"resourceType": "aws_cloudwatch_log_metric_filter",
+		"resourceName": name,
+		"searchKey": sprintf("aws_cloudwatch_log_metric_filter[%s]", [name]),
+		"issueType": "MissingAttribute",
+		"keyExpectedValue": "aws_cloudwatch_log_metric_filter should be associated an aws_cloudwatch_metric_alarm",
+		"keyActualValue": "aws_cloudwatch_log_metric_filter not associated with any aws_cloudwatch_metric_alarm",
+		"searchLine": common_lib.build_search_line(["resource","aws_cloudwatch_log_metric_filter", name], []),
+	}
+}
 
+
+check_expression_missing(filter) {
 	filter._kics_filter_expr._op == "&&"
 
-	count({x | exp := expressionArr[n]; commonLib.check_selector(filter, exp.value, exp.op, exp.name) == false; x := exp}) == 0
+	count({x | exp := expressionArr[n];
+	    common_lib.check_selector(filter, exp.value, exp.op, exp.name) == false;
+	    x := exp
+	}) == 0
 }
 
 CxPolicy[result] {
 	doc := input.document[i]
 	resources := doc.resource.aws_cloudwatch_log_metric_filter
 
-	allPatternsCount := count([x | [path, value] := walk(resources); filter := commonLib.json_unmarshal(value.pattern); x = filter])
-	count([x | [path, value] := walk(resources); filter := commonLib.json_unmarshal(value.pattern); not check_expression_missing(path[0], filter, doc); x = filter]) == allPatternsCount
-
+	resourceNames := [resourceName | [path, value] := walk(resources);
+	    filter := common_lib.json_unmarshal(value.pattern);
+	    not check_expression_missing(filter);
+	    resourceName := path[count(path)-1]
+	]
+    
+    resourceName := resourceNames[_]
+    
 	result := {
 		"documentId": input.document[i].id,
 		"resourceType": "aws_cloudwatch_log_metric_filter",
-		"resourceName": "unknown",
-		"searchKey": "resource",
+		"resourceName": resourceName,
+		"searchKey": sprintf("aws_cloudwatch_log_metric_filter.%s",[resourceName]),
 		"issueType": "MissingAttribute",
-		"keyExpectedValue": "aws_cloudwatch_log_metric_filter should have pattern { $.eventName = ConsoleLogin && $.errorMessage = \"Failed authentication\" } and be associated an aws_cloudwatch_metric_alarm",
-		"keyActualValue": "aws_cloudwatch_log_metric_filter not filtering pattern { $.eventName = ConsoleLogin && $.errorMessage = \"Failed authentication\" } or not associated with any aws_cloudwatch_metric_alarm",
-		"searchLine": commonLib.build_search_line([], []),
+		"keyExpectedValue": "aws_cloudwatch_log_metric_filter should have pattern $.eventSource equal to `s3.amazonaws.com` and $.eventName equal to `PutBucketAcl`, `PutBucketPolicy`, `PutBucketCors`, `PutBucketLifecycle`, `PutBucketReplication`, `DeleteBucketPolicy`, `DeleteBucketCors`, `DeleteBucketLifecycle` and `DeleteBucketReplication`",
+		"keyActualValue": "aws_cloudwatch_log_metric_filter with wrong pattern",
+		"searchLine": common_lib.build_search_line(["resource","aws_cloudwatch_log_metric_filter", resourceName, "pattern"], []),
 	}
-}
+}
\ No newline at end of file


Test patch:
diff --git a/assets/queries/terraform/aws/cloudwatch_s3_policy_change_alarm_missing/test/negative1.tf b/assets/queries/terraform/aws/cloudwatch_s3_policy_change_alarm_missing/test/negative1.tf
index 946b52780c1..b1de8c283e9 100644
--- a/assets/queries/terraform/aws/cloudwatch_s3_policy_change_alarm_missing/test/negative1.tf
+++ b/assets/queries/terraform/aws/cloudwatch_s3_policy_change_alarm_missing/test/negative1.tf
@@ -26,7 +26,7 @@ resource "aws_cloudwatch_metric_alarm" "cis_s3_bucket_policy_change_cw_alarm" {
 
 resource "aws_cloudwatch_log_metric_filter" "cis_no_mfa_console_signin_metric_filter" {
   name           = "CIS-ConsoleSigninWithoutMFA"
-  pattern        = "{ ($.eventName = \"ConsoleLogin\") && ($.additionalEventData.MFAUsed != \"Yes\") }"
+  pattern        = "{ ($.eventSource = \"s3.amazonaws.com\") && (($.eventName = PutBucketAcl) || ($.eventName = PutBucketPolicy) || ($.eventName = PutBucketCors) || ($.eventName = PutBucketLifecycle) || ($.eventName = PutBucketReplication) || ($.eventName = DeleteBucketPolicy) || ($.eventName = DeleteBucketCors) || ($.eventName = DeleteBucketLifecycle) || ($.eventName = DeleteBucketReplication)) }"
   log_group_name = aws_cloudwatch_log_group.CIS_CloudWatch_LogsGroup.name
 
   metric_transformation {
diff --git a/assets/queries/terraform/aws/cloudwatch_s3_policy_change_alarm_missing/test/positive5.tf b/assets/queries/terraform/aws/cloudwatch_s3_policy_change_alarm_missing/test/positive5.tf
new file mode 100644
index 00000000000..bbba966c2c2
--- /dev/null
+++ b/assets/queries/terraform/aws/cloudwatch_s3_policy_change_alarm_missing/test/positive5.tf
@@ -0,0 +1,53 @@
+resource "aws_cloudwatch_log_metric_filter" "cis_s3_bucket_policy_change_metric_filter" {
+  name           = "CIS-S3BucketPolicyChanges"
+  #                                                    Here should be &&
+  pattern        = "{ ($.eventSource = \"s3.amazonaws.com\") || (($.eventName = PutBucketAcl) || ($.eventName = PutBucketPolicy) || ($.eventName = PutBucketCors) || ($.eventName = PutBucketLifecycle) || ($.eventName = PutBucketReplication) || ($.eventName = DeleteBucketPolicy) || ($.eventName = DeleteBucketCors) || ($.eventName = DeleteBucketLifecycle) || ($.eventName = DeleteBucketReplication)) }"
+  log_group_name = aws_cloudwatch_log_group.CIS_CloudWatch_LogsGroup.name
+
+  metric_transformation {
+    name      = "CIS-S3BucketPolicyChanges"
+    namespace = "CIS_Metric_Alarm_Namespace"
+    value     = "1"
+  }
+}
+
+resource "aws_cloudwatch_metric_alarm" "cis_s3_bucket_policy_change_cw_alarm" {
+  alarm_name                = "CIS-3.8-S3BucketPolicyChanges"
+  comparison_operator       = "GreaterThanOrEqualToThreshold"
+  evaluation_periods        = "1"
+  metric_name               = aws_cloudwatch_log_metric_filter.cis_s3_bucket_policy_change_metric_filter.id
+  namespace                 = "CIS_Metric_Alarm_Namespace"
+  period                    = "300"
+  statistic                 = "Sum"
+  threshold                 = "1"
+  alarm_description         = "Monitoring changes to S3 bucket policies may reduce time to detect and correct permissive policies on sensitive S3 buckets."
+  alarm_actions             = [aws_sns_topic.CIS_Alerts_SNS_Topic.arn]
+  insufficient_data_actions = []
+}
+
+
+resource "aws_cloudwatch_log_metric_filter" "cis_no_mfa_console_signin_metric_filter" {
+  name           = "CIS-ConsoleSigninWithoutMFA"
+  pattern        = "{ ($.eventName = \"ConsoleLogin\") && ($.additionalEventData.MFAUsed != \"Yes\") }"
+  log_group_name = aws_cloudwatch_log_group.CIS_CloudWatch_LogsGroup.name
+
+  metric_transformation {
+    name      = "CIS-ConsoleSigninWithoutMFA"
+    namespace = "CIS_Metric_Alarm_Namespace"
+    value     = "1"
+  }
+}
+
+resource "aws_cloudwatch_metric_alarm" "cis_no_mfa_console_signin_cw_alarm" {
+  alarm_name                = "CIS-3.2-ConsoleSigninWithoutMFA"
+  comparison_operator       = "GreaterThanOrEqualToThreshold"
+  evaluation_periods        = "1"
+  metric_name               = aws_cloudwatch_log_metric_filter.cis_no_mfa_console_signin_metric_filter.id
+  namespace                 = "CIS_Metric_Alarm_Namespace"
+  period                    = "300"
+  statistic                 = "Sum"
+  threshold                 = "1"
+  alarm_description         = "Monitoring for single-factor console logins will increase visibility into accounts that are not protected by MFA."
+  alarm_actions             = [aws_sns_topic.CIS_Alerts_SNS_Topic.arn]
+  insufficient_data_actions = []
+}
diff --git a/assets/queries/terraform/aws/cloudwatch_s3_policy_change_alarm_missing/test/positive_expected_result.json b/assets/queries/terraform/aws/cloudwatch_s3_policy_change_alarm_missing/test/positive_expected_result.json
index 3ecc35424a8..6e6bdca530c 100644
--- a/assets/queries/terraform/aws/cloudwatch_s3_policy_change_alarm_missing/test/positive_expected_result.json
+++ b/assets/queries/terraform/aws/cloudwatch_s3_policy_change_alarm_missing/test/positive_expected_result.json
@@ -8,19 +8,43 @@
   {
     "queryName": "CloudWatch S3 policy Change Alarm Missing",
     "severity": "MEDIUM",
-    "line": 1,
+    "line": 3,
+    "fileName": "positive1.tf"
+  },
+  {
+    "queryName": "CloudWatch S3 policy Change Alarm Missing",
+    "severity": "MEDIUM",
+    "line": 30,
+    "fileName": "positive1.tf"
+  },
+  {
+    "queryName": "CloudWatch S3 policy Change Alarm Missing",
+    "severity": "MEDIUM",
+    "line": 3,
     "fileName": "positive2.tf"
   },
   {
     "queryName": "CloudWatch S3 policy Change Alarm Missing",
     "severity": "MEDIUM",
-    "line": 1,
+    "line": 3,
     "fileName": "positive3.tf"
   },
   {
     "queryName": "CloudWatch S3 policy Change Alarm Missing",
     "severity": "MEDIUM",
-    "line": 1,
+    "line": 3,
     "fileName": "positive4.tf"
+  },
+  {
+    "queryName": "CloudWatch S3 policy Change Alarm Missing",
+    "severity": "MEDIUM",
+    "line": 4,
+    "fileName": "positive5.tf"
+  },
+  {
+    "queryName": "CloudWatch S3 policy Change Alarm Missing",
+    "severity": "MEDIUM",
+    "line": 31,
+    "fileName": "positive5.tf"
   }
 ]