fix: Fix docker rego bug where same FROM ref causes crash (#707)

* fix: Fix docker rego bug where same FROM ref causes crash
aquasecurity · Jun 24, 2022 · 8560a5c · 8560a5c
1 parent e02f93a
commit 8560a5c
Show file tree

Hide file tree

Showing 11 changed files with 52 additions and 12 deletions.
diff --git a/avd_docs/rbac/general/AVD-KSV-0044/docs.md b/avd_docs/rbac/general/AVD-KSV-0044/docs.md
@@ -1,5 +1,5 @@
 
-Check whether role permits any verb on any resource
+Check whether role permits wildcard verb on wildcard resource
 
 ### Impact
 <!-- Add Impact here -->

diff --git a/avd_docs/rbac/general/AVD-KSV-0045/docs.md b/avd_docs/rbac/general/AVD-KSV-0045/docs.md
@@ -1,5 +1,5 @@
 
-Check whether role permits any verb on specific resources
+Check whether role permits wildcard verb on specific resources
 
 ### Impact
 <!-- Add Impact here -->

diff --git a/avd_docs/rbac/general/AVD-KSV-0046/docs.md b/avd_docs/rbac/general/AVD-KSV-0046/docs.md
@@ -1,5 +1,5 @@
 
-Check whether role permits specific verb on any resources
+Check whether role permits specific verb on wildcard resources
 
 ### Impact
 <!-- Add Impact here -->

diff --git a/avd_docs/rbac/general/AVD-KSV-0048/docs.md b/avd_docs/rbac/general/AVD-KSV-0048/docs.md
@@ -1,5 +1,5 @@
 
-Check whether role permits update/create a malicious pod
+Check whether role permits update/create of a malicious pod
 
 ### Impact
 <!-- Add Impact here -->

diff --git a/avd_docs/rbac/general/AVD-KSV-0050/docs.md b/avd_docs/rbac/general/AVD-KSV-0050/docs.md
@@ -1,5 +1,5 @@
 
-An effective level of access equivalent to cluster-admin.
+An effective level of access equivalent to cluster-admin should not be provided.
 
 ### Impact
 <!-- Add Impact here -->

diff --git a/avd_docs/rbac/general/AVD-KSV-0051/docs.md b/avd_docs/rbac/general/AVD-KSV-0051/docs.md
@@ -1,5 +1,5 @@
 
-Check whether role permits creating role binding and associate to privileged role/clusterrole
+Check whether role permits creating role bindings and associating to privileged role/clusterrole
 
 ### Impact
 <!-- Add Impact here -->

diff --git a/avd_docs/rbac/general/AVD-KSV-0052/docs.md b/avd_docs/rbac/general/AVD-KSV-0052/docs.md
@@ -1,5 +1,5 @@
 
-Check whether role permits creating role clusterrolebindings and associate to privileged cluster role
+Check whether role permits creating role ClusterRoleBindings and association with privileged cluster role
 
 ### Impact
 <!-- Add Impact here -->

diff --git a/avd_docs/rbac/general/AVD-KSV-0054/docs.md b/avd_docs/rbac/general/AVD-KSV-0054/docs.md
@@ -1,5 +1,5 @@
 
-Check whether role permits attaching shell on pods
+Check whether role permits attaching to shell on pods
 
 ### Impact
 <!-- Add Impact here -->

diff --git a/internal/rules/docker/lib/docker.rego b/internal/rules/docker/lib/docker.rego
@@ -20,9 +20,8 @@ copy[instruction] {
 	instruction.Cmd == "copy"
 }
 
-stage_copies[stage_name] = copies {
+stage_copies[stage] = copies {
 	stage := input.Stages[_]
-	stage_name := stage.Name
 	copies := [copy | copy := stage.Commands[_]; copy.Cmd == "copy"]
 }
 

diff --git a/internal/rules/docker/policies/copy_from_references_current_from_alias.rego b/internal/rules/docker/policies/copy_from_references_current_from_alias.rego
@@ -10,7 +10,7 @@ __rego_metadata__ := {
 	"severity": "CRITICAL",
 	"type": "Dockerfile Security Check",
 	"description": "COPY '--from' should not mention the current FROM alias, since it is impossible to copy from itself.",
-	"recommended_actions": "Change the '--form' so that it will not refer to itself",
+	"recommended_actions": "Change the '--from' so that it will not refer to itself",
 	"url": "https://docs.docker.com/develop/develop-images/multistage-build/",
 }
 
@@ -27,7 +27,7 @@ get_alias_from_copy[output] {
 	contains(flag, "--from=")
 	parts := split(flag, "=")
 
-	is_alias_current_from_alias(stage, parts[1])
+	is_alias_current_from_alias(stage.Name, parts[1])
 	args := parts[1]
 	output := {
 		"args": args,

diff --git a/internal/rules/docker/policies/copy_from_references_current_from_alias_test.rego b/internal/rules/docker/policies/copy_from_references_current_from_alias_test.rego
@@ -155,3 +155,44 @@ test_basic_allowed {
 
 	count(r) == 0
 }
+
+test_duplicate_allowed {
+	r := deny with input as {"Stages": [
+		{
+			"Name": "golang:1.7.3",
+			"Commands": [
+				{
+					"Cmd": "from",
+					"Value": ["golang:1.7.3"],
+				},
+				{
+					"Cmd": "copy",
+					"Flags": ["--from=dep"],
+					"Value": [
+						"/binary",
+						"/",
+					],
+				},
+			],
+		},
+		{
+			"Name": "golang:1.7.3",
+			"Commands": [
+				{
+					"Cmd": "from",
+					"Value": ["golang:1.7.3"],
+				},
+				{
+					"Cmd": "copy",
+					"Flags": ["--from=0"],
+					"Value": [
+						"app/",
+						"/app/",
+					],
+				},
+			],
+		},
+	]}
+
+	count(r) == 0
+}