From 7fda9d786a88d5cf9f473174573c6bade24ea12a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Felicidade?= Date: Mon, 2 May 2022 15:45:54 +0100 Subject: [PATCH 1/6] issues 5115 5116 5118 --- .../metadata.json | 4 +-- .../query.rego | 25 +++++++++++++++-- ...gative.dockerfile => negative1.dockerfile} | 2 +- .../test/negative2.dockerfile | 11 ++++++++ ...sitive.dockerfile => negative3.dockerfile} | 0 .../test/negative4.dockerfile | 11 ++++++++ .../test/positive1.dockerfile | 11 ++++++++ .../test/positive2.dockerfile | 11 ++++++++ .../test/positive_expected_result.json | 11 ++++++-- .../metadata.json | 2 +- .../query.rego | 28 ++++++------------- ...gative.dockerfile => negative1.dockerfile} | 0 .../test/positive.dockerfile | 12 -------- .../test/positive1.dockerfile | 11 ++++++++ .../test/positive_expected_result.json | 3 +- .../metadata.json | 2 +- 16 files changed, 102 insertions(+), 42 deletions(-) rename assets/queries/dockerfile/changing_default_shell_using_shell_command/test/{negative.dockerfile => negative1.dockerfile} (80%) create mode 100644 assets/queries/dockerfile/changing_default_shell_using_shell_command/test/negative2.dockerfile rename assets/queries/dockerfile/changing_default_shell_using_shell_command/test/{positive.dockerfile => negative3.dockerfile} (100%) create mode 100644 assets/queries/dockerfile/changing_default_shell_using_shell_command/test/negative4.dockerfile create mode 100644 assets/queries/dockerfile/changing_default_shell_using_shell_command/test/positive1.dockerfile create mode 100644 assets/queries/dockerfile/changing_default_shell_using_shell_command/test/positive2.dockerfile rename assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/test/{negative.dockerfile => negative1.dockerfile} (100%) delete mode 100644 assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/test/positive.dockerfile create mode 100644 assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/test/positive1.dockerfile diff --git a/assets/queries/dockerfile/changing_default_shell_using_shell_command/metadata.json b/assets/queries/dockerfile/changing_default_shell_using_shell_command/metadata.json index e6bc21bb0a3..a138e3b0e4b 100644 --- a/assets/queries/dockerfile/changing_default_shell_using_shell_command/metadata.json +++ b/assets/queries/dockerfile/changing_default_shell_using_shell_command/metadata.json @@ -1,9 +1,9 @@ { "id": "8a301064-c291-4b20-adcb-403fe7fd95fd", - "queryName": "Changing Default Shell Using SHELL Command", + "queryName": "Changing Default Shell Using RUN Command", "severity": "MEDIUM", "category": "Insecure Defaults", - "descriptionText": "Using the command SHELL to override the default shell instead of the RUN command", + "descriptionText": "Using the command RUN to override the default shell instead of the SHELL command leads to ineficiencies. It also does not make sense since Docker provides the SHELL command for this exact purpose.", "descriptionUrl": "https://docs.docker.com/engine/reference/builder/#shell", "platform": "Dockerfile", "descriptionID": "d859b2eb" diff --git a/assets/queries/dockerfile/changing_default_shell_using_shell_command/query.rego b/assets/queries/dockerfile/changing_default_shell_using_shell_command/query.rego index bfbad039d76..1232ad9694f 100644 --- a/assets/queries/dockerfile/changing_default_shell_using_shell_command/query.rego +++ b/assets/queries/dockerfile/changing_default_shell_using_shell_command/query.rego @@ -1,14 +1,33 @@ package Cx +import data.generic.common as common_lib + +CxPolicy[result] { + resource := input.document[i].command[name][_] + resource.Cmd == "run" + value := resource.Value + contains(value[v], "/bin/bash") + + result := { + "documentId": input.document[i].id, + "searchKey": sprintf("FROM={{%s}}.{{%s}}", [name, resource.Original]), + "issueType": "IncorrectValue", + "keyExpectedValue": sprintf("FROM={{%s}}.{{%s}} uses the SHELL command to change the default shell", [name, resource.Original]), + "keyActualValue": sprintf("FROM={{%s}}.{{%s}} uses the RUN command to change the default shell", [name, resource.Original]), + } +} + CxPolicy[result] { resource := input.document[i].command[name][_] - resource.Cmd == "shell" + resource.Cmd == "run" + value := resource.Value + contains(value[v], "powershell") result := { "documentId": input.document[i].id, "searchKey": sprintf("FROM={{%s}}.{{%s}}", [name, resource.Original]), "issueType": "IncorrectValue", - "keyExpectedValue": sprintf("FROM={{%s}}.{{%s}} uses the RUN command to change the default shell", [name, resource.Original]), - "keyActualValue": sprintf("FROM={{%s}}.{{%s}} uses the SHELL command to change the default shell", [name, resource.Original]), + "keyExpectedValue": sprintf("FROM={{%s}}.{{%s}} uses the SHELL command to change the default shell", [name, resource.Original]), + "keyActualValue": sprintf("FROM={{%s}}.{{%s}} uses the RUN command to change the default shell", [name, resource.Original]), } } diff --git a/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/negative.dockerfile b/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/negative1.dockerfile similarity index 80% rename from assets/queries/dockerfile/changing_default_shell_using_shell_command/test/negative.dockerfile rename to assets/queries/dockerfile/changing_default_shell_using_shell_command/test/negative1.dockerfile index 2c6c823cd61..f94f4356f19 100644 --- a/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/negative.dockerfile +++ b/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/negative1.dockerfile @@ -2,7 +2,7 @@ FROM alpine:3.5 RUN apk add --update py2-pip RUN sudo yum install -y bundler RUN yum install -RUN ["powershell", "-command", "Execute-MyCmdlet", "-param1 "c:\foo.txt""] +SHELL ["/bin/bash", "-c"] COPY requirements.txt /usr/src/app/ RUN pip install --no-cache-dir -r /usr/src/app/requirements.txt COPY app.py /usr/src/app/ diff --git a/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/negative2.dockerfile b/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/negative2.dockerfile new file mode 100644 index 00000000000..ff58ecf92d3 --- /dev/null +++ b/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/negative2.dockerfile @@ -0,0 +1,11 @@ +FROM alpine:3.5 +RUN apk add --update py2-pip +RUN sudo yum install -y bundler +RUN yum install +SHELL ["cmd", "/S", "/C"] +COPY requirements.txt /usr/src/app/ +RUN pip install --no-cache-dir -r /usr/src/app/requirements.txt +COPY app.py /usr/src/app/ +COPY templates/index.html /usr/src/app/templates/ +EXPOSE 5000 +CMD ["python", "/usr/src/app/app.py"] diff --git a/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/positive.dockerfile b/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/negative3.dockerfile similarity index 100% rename from assets/queries/dockerfile/changing_default_shell_using_shell_command/test/positive.dockerfile rename to assets/queries/dockerfile/changing_default_shell_using_shell_command/test/negative3.dockerfile diff --git a/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/negative4.dockerfile b/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/negative4.dockerfile new file mode 100644 index 00000000000..18b828c94de --- /dev/null +++ b/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/negative4.dockerfile @@ -0,0 +1,11 @@ +FROM alpine:3.5 +RUN apk add --update py2-pip +RUN sudo yum install -y bundler +RUN yum install +SHELL ["/bin/sh", "-c"] +COPY requirements.txt /usr/src/app/ +RUN pip install --no-cache-dir -r /usr/src/app/requirements.txt +COPY app.py /usr/src/app/ +COPY templates/index.html /usr/src/app/templates/ +EXPOSE 5000 +CMD ["python", "/usr/src/app/app.py"] diff --git a/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/positive1.dockerfile b/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/positive1.dockerfile new file mode 100644 index 00000000000..ab7639f03c3 --- /dev/null +++ b/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/positive1.dockerfile @@ -0,0 +1,11 @@ +FROM alpine:3.5 +RUN apk add --update py2-pip +RUN sudo yum install -y bundler +RUN yum install +RUN ln -sfv /bin/bash /bin/sh +COPY requirements.txt /usr/src/app/ +RUN pip install --no-cache-dir -r /usr/src/app/requirements.txt +COPY app.py /usr/src/app/ +COPY templates/index.html /usr/src/app/templates/ +EXPOSE 5000 +CMD ["python", "/usr/src/app/app.py"] diff --git a/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/positive2.dockerfile b/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/positive2.dockerfile new file mode 100644 index 00000000000..b81cb2e145f --- /dev/null +++ b/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/positive2.dockerfile @@ -0,0 +1,11 @@ +FROM alpine:3.5 +RUN apk add --update py2-pip +RUN sudo yum install -y bundler +RUN yum install +RUN powershell -command +COPY requirements.txt /usr/src/app/ +RUN pip install --no-cache-dir -r /usr/src/app/requirements.txt +COPY app.py /usr/src/app/ +COPY templates/index.html /usr/src/app/templates/ +EXPOSE 5000 +CMD ["python", "/usr/src/app/app.py"] diff --git a/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/positive_expected_result.json b/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/positive_expected_result.json index 0fbbc5c68db..c9854941220 100644 --- a/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/positive_expected_result.json +++ b/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/positive_expected_result.json @@ -1,7 +1,14 @@ [ { - "queryName": "Changing Default Shell Using SHELL Command", + "queryName": "Changing Default Shell Using RUN Command", "severity": "MEDIUM", - "line": 5 + "line": 5, + "filename": "positive1.dockerfile" + }, + { + "queryName": "Changing Default Shell Using RUN Command", + "severity": "MEDIUM", + "line": 5, + "filename": "positive2.dockerfile" } ] diff --git a/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/metadata.json b/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/metadata.json index 9ea6d8e430f..709d085c489 100644 --- a/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/metadata.json +++ b/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/metadata.json @@ -3,7 +3,7 @@ "queryName": "COPY '--from' Without FROM Alias Defined Previously", "severity": "MEDIUM", "category": "Build Process", - "descriptionText": "COPY command with the flag '--from' should mention a previously defined FROM alias", + "descriptionText": " This query is used to ensure that build stages are named. This way even if the Dockerfile is re-ordered, the COPY instruction doesn’t break.", "descriptionUrl": "https://docs.docker.com/develop/develop-images/multistage-build/", "platform": "Dockerfile", "descriptionID": "dea09829" diff --git a/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/query.rego b/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/query.rego index 4bdb470b56b..d4964ae8e08 100644 --- a/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/query.rego +++ b/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/query.rego @@ -1,31 +1,21 @@ package Cx CxPolicy[result] { - resource := input.document[i].command[name][_] - resource.Cmd == "copy" - contains(resource.Flags[x], "--from=") - resource.Flags[x] != "--from=0" - aux_split := split(resource.Flags[x], "=") - - not isPreviousAlias(resource._kics_line, aux_split[1]) + commands := input.document[i].command[name][_] + + commands.Cmd == "copy" + flags := commands.Flags + contains(flags[f], "--from=") + flag_split := split(flags[f], "=") + to_number(flag_split[1]) > -1 + result := { "documentId": input.document[i].id, - "searchKey": sprintf("FROM={{%s}}.{{%s}}", [name, resource.Original]), + "searchKey": sprintf("FROM={{%s}}.{{%s}}", [name, commands.Original]), "issueType": "IncorrectValue", "keyExpectedValue": "COPY '--from' references a previous defined FROM alias", "keyActualValue": "COPY '--from' does not reference a previous defined FROM alias", } } - -isPreviousAlias(startLine, currentAlias) = allow { - resource := input.document[i].command[name][_] - resource.Cmd == "from" - previousAlias := resource.Value[2] - - previousAlias == currentAlias - resource.EndLine < startLine - - allow = true -} diff --git a/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/test/negative.dockerfile b/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/test/negative1.dockerfile similarity index 100% rename from assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/test/negative.dockerfile rename to assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/test/negative1.dockerfile diff --git a/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/test/positive.dockerfile b/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/test/positive.dockerfile deleted file mode 100644 index 1132a415898..00000000000 --- a/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/test/positive.dockerfile +++ /dev/null @@ -1,12 +0,0 @@ -FROM golang:1.7.3 AS builder -WORKDIR /go/src/github.com/alexellis/href-counter/ -RUN go get -d -v golang.org/x/net/html -COPY app.go . -RUN CGO_ENABLED=0 GOOS=linux go build -a -installsuffix cgo -o app . - - -FROM alpine:latest -RUN apk --no-cache add ca-certificates -WORKDIR /root/ -COPY --from=builder2 /go/src/github.com/alexellis/href-counter/app . -CMD ["./app"] \ No newline at end of file diff --git a/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/test/positive1.dockerfile b/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/test/positive1.dockerfile new file mode 100644 index 00000000000..5ff63e3c9d7 --- /dev/null +++ b/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/test/positive1.dockerfile @@ -0,0 +1,11 @@ +FROM golang:1.16 +WORKDIR /go/src/github.com/alexellis/href-counter/ +RUN go get -d -v golang.org/x/net/html +COPY app.go ./ +RUN CGO_ENABLED=0 GOOS=linux go build -a -installsuffix cgo -o app . + +FROM alpine:latest +RUN apk --no-cache add ca-certificates +WORKDIR /root/ +COPY --from=0 /go/src/github.com/alexellis/href-counter/app ./ +CMD ["./app"] diff --git a/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/test/positive_expected_result.json b/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/test/positive_expected_result.json index aae698b5213..139bfda874c 100644 --- a/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/test/positive_expected_result.json +++ b/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/test/positive_expected_result.json @@ -2,6 +2,7 @@ { "queryName": "COPY '--from' Without FROM Alias Defined Previously", "severity": "MEDIUM", - "line": 11 + "line": 10, + "filename": "positive1.dockerfile" } ] diff --git a/assets/queries/dockerfile/run_command_cd_instead_of_workdir/metadata.json b/assets/queries/dockerfile/run_command_cd_instead_of_workdir/metadata.json index eca2a3750ee..9b89cccffc1 100644 --- a/assets/queries/dockerfile/run_command_cd_instead_of_workdir/metadata.json +++ b/assets/queries/dockerfile/run_command_cd_instead_of_workdir/metadata.json @@ -3,7 +3,7 @@ "queryName": "RUN Instruction Using 'cd' Instead of WORKDIR", "severity": "MEDIUM", "category": "Build Process", - "descriptionText": "Use WORKDIR instead of proliferating instructions like RUN cd \u2026 && do-something, which are hard to read, troubleshoot, and maintain.", + "descriptionText": "The user can utilize WORKDIR or the full path instead of proliferating instructions like RUN cd \u2026 && do-something, which are hard to read, troubleshoot, and maintain. Although using the full path can also make the code harder to read sometimes, it is preferable.", "descriptionUrl": "https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#workdir", "platform": "Dockerfile", "descriptionID": "edd9f7d3" From beb6453ff138e00a597a71a7d0522b2a3cdc6772 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Felicidade?= Date: Fri, 6 May 2022 12:27:14 +0100 Subject: [PATCH 2/6] re-refactored query for changing shell, renamed copy from query --- .../metadata.json | 4 ++-- .../query.rego | 16 ++++++++++++---- .../test/negative1.dockerfile | 0 .../test/negative2.dockerfile | 0 .../test/negative3.dockerfile | 0 .../test/negative4.dockerfile | 0 .../test/positive1.dockerfile | 0 .../test/positive2.dockerfile | 0 .../test/positive_expected_result.json | 0 .../metadata.json | 2 +- .../query.rego | 4 ++-- .../test/negative1.dockerfile | 0 .../test/positive1.dockerfile | 0 .../test/positive_expected_result.json | 2 +- 14 files changed, 18 insertions(+), 10 deletions(-) rename assets/queries/dockerfile/{changing_default_shell_using_shell_command => changing_default_shell_using_run_command}/metadata.json (63%) rename assets/queries/dockerfile/{changing_default_shell_using_shell_command => changing_default_shell_using_run_command}/query.rego (53%) rename assets/queries/dockerfile/{changing_default_shell_using_shell_command => changing_default_shell_using_run_command}/test/negative1.dockerfile (100%) rename assets/queries/dockerfile/{changing_default_shell_using_shell_command => changing_default_shell_using_run_command}/test/negative2.dockerfile (100%) rename assets/queries/dockerfile/{changing_default_shell_using_shell_command => changing_default_shell_using_run_command}/test/negative3.dockerfile (100%) rename assets/queries/dockerfile/{changing_default_shell_using_shell_command => changing_default_shell_using_run_command}/test/negative4.dockerfile (100%) rename assets/queries/dockerfile/{changing_default_shell_using_shell_command => changing_default_shell_using_run_command}/test/positive1.dockerfile (100%) rename assets/queries/dockerfile/{changing_default_shell_using_shell_command => changing_default_shell_using_run_command}/test/positive2.dockerfile (100%) rename assets/queries/dockerfile/{changing_default_shell_using_shell_command => changing_default_shell_using_run_command}/test/positive_expected_result.json (100%) rename assets/queries/dockerfile/{copy_from_without_from_alias_defined_previously => using_unnamed_build_stages}/metadata.json (85%) rename assets/queries/dockerfile/{copy_from_without_from_alias_defined_previously => using_unnamed_build_stages}/query.rego (80%) rename assets/queries/dockerfile/{copy_from_without_from_alias_defined_previously => using_unnamed_build_stages}/test/negative1.dockerfile (100%) rename assets/queries/dockerfile/{copy_from_without_from_alias_defined_previously => using_unnamed_build_stages}/test/positive1.dockerfile (100%) rename assets/queries/dockerfile/{copy_from_without_from_alias_defined_previously => using_unnamed_build_stages}/test/positive_expected_result.json (54%) diff --git a/assets/queries/dockerfile/changing_default_shell_using_shell_command/metadata.json b/assets/queries/dockerfile/changing_default_shell_using_run_command/metadata.json similarity index 63% rename from assets/queries/dockerfile/changing_default_shell_using_shell_command/metadata.json rename to assets/queries/dockerfile/changing_default_shell_using_run_command/metadata.json index a138e3b0e4b..0714beb7ec5 100644 --- a/assets/queries/dockerfile/changing_default_shell_using_shell_command/metadata.json +++ b/assets/queries/dockerfile/changing_default_shell_using_run_command/metadata.json @@ -2,8 +2,8 @@ "id": "8a301064-c291-4b20-adcb-403fe7fd95fd", "queryName": "Changing Default Shell Using RUN Command", "severity": "MEDIUM", - "category": "Insecure Defaults", - "descriptionText": "Using the command RUN to override the default shell instead of the SHELL command leads to ineficiencies. It also does not make sense since Docker provides the SHELL command for this exact purpose.", + "category": "Best Practices", + "descriptionText": "Using the command RUN to override the default shell instead of the SHELL command leads to inefficiencies. It also does not make sense since Docker provides the SHELL command for this exact purpose.", "descriptionUrl": "https://docs.docker.com/engine/reference/builder/#shell", "platform": "Dockerfile", "descriptionID": "d859b2eb" diff --git a/assets/queries/dockerfile/changing_default_shell_using_shell_command/query.rego b/assets/queries/dockerfile/changing_default_shell_using_run_command/query.rego similarity index 53% rename from assets/queries/dockerfile/changing_default_shell_using_shell_command/query.rego rename to assets/queries/dockerfile/changing_default_shell_using_run_command/query.rego index 1232ad9694f..a141507b89d 100644 --- a/assets/queries/dockerfile/changing_default_shell_using_shell_command/query.rego +++ b/assets/queries/dockerfile/changing_default_shell_using_run_command/query.rego @@ -6,13 +6,19 @@ CxPolicy[result] { resource := input.document[i].command[name][_] resource.Cmd == "run" value := resource.Value - contains(value[v], "/bin/bash") + shell_possibilities := {"/bin/bash", "/bin/tcsh", " /bin/ksh", "/bin/csh", "/bin/dash" , "etc/shells", "/zsh", "/bin/fish", "/bin/tmux", "/bin/rbash"} + contains(value[v], shell_possibilities[p]) + run_values := split(value[v], " ") + command := run_values[0] + command_possibilities := {"mv", "chsh", "usermod", "ln"} + command == command_possibilities[cp] result := { + "debug": sprintf("%s", [value[v]]), "documentId": input.document[i].id, "searchKey": sprintf("FROM={{%s}}.{{%s}}", [name, resource.Original]), "issueType": "IncorrectValue", - "keyExpectedValue": sprintf("FROM={{%s}}.{{%s}} uses the SHELL command to change the default shell", [name, resource.Original]), + "keyExpectedValue": sprintf("FROM={{%s}}.{{%s}} should use the SHELL command to change the default shell", [name, resource.Original]), "keyActualValue": sprintf("FROM={{%s}}.{{%s}} uses the RUN command to change the default shell", [name, resource.Original]), } } @@ -21,13 +27,15 @@ CxPolicy[result] { resource := input.document[i].command[name][_] resource.Cmd == "run" value := resource.Value - contains(value[v], "powershell") + run_values := split(value[v], " ") + command := run_values[0] + contains(command, "powershell") result := { "documentId": input.document[i].id, "searchKey": sprintf("FROM={{%s}}.{{%s}}", [name, resource.Original]), "issueType": "IncorrectValue", - "keyExpectedValue": sprintf("FROM={{%s}}.{{%s}} uses the SHELL command to change the default shell", [name, resource.Original]), + "keyExpectedValue": sprintf("FROM={{%s}}.{{%s}} should use the SHELL command to change the default shell", [name, resource.Original]), "keyActualValue": sprintf("FROM={{%s}}.{{%s}} uses the RUN command to change the default shell", [name, resource.Original]), } } diff --git a/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/negative1.dockerfile b/assets/queries/dockerfile/changing_default_shell_using_run_command/test/negative1.dockerfile similarity index 100% rename from assets/queries/dockerfile/changing_default_shell_using_shell_command/test/negative1.dockerfile rename to assets/queries/dockerfile/changing_default_shell_using_run_command/test/negative1.dockerfile diff --git a/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/negative2.dockerfile b/assets/queries/dockerfile/changing_default_shell_using_run_command/test/negative2.dockerfile similarity index 100% rename from assets/queries/dockerfile/changing_default_shell_using_shell_command/test/negative2.dockerfile rename to assets/queries/dockerfile/changing_default_shell_using_run_command/test/negative2.dockerfile diff --git a/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/negative3.dockerfile b/assets/queries/dockerfile/changing_default_shell_using_run_command/test/negative3.dockerfile similarity index 100% rename from assets/queries/dockerfile/changing_default_shell_using_shell_command/test/negative3.dockerfile rename to assets/queries/dockerfile/changing_default_shell_using_run_command/test/negative3.dockerfile diff --git a/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/negative4.dockerfile b/assets/queries/dockerfile/changing_default_shell_using_run_command/test/negative4.dockerfile similarity index 100% rename from assets/queries/dockerfile/changing_default_shell_using_shell_command/test/negative4.dockerfile rename to assets/queries/dockerfile/changing_default_shell_using_run_command/test/negative4.dockerfile diff --git a/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/positive1.dockerfile b/assets/queries/dockerfile/changing_default_shell_using_run_command/test/positive1.dockerfile similarity index 100% rename from assets/queries/dockerfile/changing_default_shell_using_shell_command/test/positive1.dockerfile rename to assets/queries/dockerfile/changing_default_shell_using_run_command/test/positive1.dockerfile diff --git a/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/positive2.dockerfile b/assets/queries/dockerfile/changing_default_shell_using_run_command/test/positive2.dockerfile similarity index 100% rename from assets/queries/dockerfile/changing_default_shell_using_shell_command/test/positive2.dockerfile rename to assets/queries/dockerfile/changing_default_shell_using_run_command/test/positive2.dockerfile diff --git a/assets/queries/dockerfile/changing_default_shell_using_shell_command/test/positive_expected_result.json b/assets/queries/dockerfile/changing_default_shell_using_run_command/test/positive_expected_result.json similarity index 100% rename from assets/queries/dockerfile/changing_default_shell_using_shell_command/test/positive_expected_result.json rename to assets/queries/dockerfile/changing_default_shell_using_run_command/test/positive_expected_result.json diff --git a/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/metadata.json b/assets/queries/dockerfile/using_unnamed_build_stages/metadata.json similarity index 85% rename from assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/metadata.json rename to assets/queries/dockerfile/using_unnamed_build_stages/metadata.json index 709d085c489..4d778e4a998 100644 --- a/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/metadata.json +++ b/assets/queries/dockerfile/using_unnamed_build_stages/metadata.json @@ -1,6 +1,6 @@ { "id": "68a51e22-ae5a-4d48-8e87-b01a323605c9", - "queryName": "COPY '--from' Without FROM Alias Defined Previously", + "queryName": "Using Unnamed Build Stages", "severity": "MEDIUM", "category": "Build Process", "descriptionText": " This query is used to ensure that build stages are named. This way even if the Dockerfile is re-ordered, the COPY instruction doesn’t break.", diff --git a/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/query.rego b/assets/queries/dockerfile/using_unnamed_build_stages/query.rego similarity index 80% rename from assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/query.rego rename to assets/queries/dockerfile/using_unnamed_build_stages/query.rego index d4964ae8e08..ff621dc15f8 100644 --- a/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/query.rego +++ b/assets/queries/dockerfile/using_unnamed_build_stages/query.rego @@ -15,7 +15,7 @@ CxPolicy[result] { "documentId": input.document[i].id, "searchKey": sprintf("FROM={{%s}}.{{%s}}", [name, commands.Original]), "issueType": "IncorrectValue", - "keyExpectedValue": "COPY '--from' references a previous defined FROM alias", - "keyActualValue": "COPY '--from' does not reference a previous defined FROM alias", + "keyExpectedValue": "COPY '--from' should reference a previously defined FROM alias", + "keyActualValue": "COPY '--from' does not reference a previously defined FROM alias", } } diff --git a/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/test/negative1.dockerfile b/assets/queries/dockerfile/using_unnamed_build_stages/test/negative1.dockerfile similarity index 100% rename from assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/test/negative1.dockerfile rename to assets/queries/dockerfile/using_unnamed_build_stages/test/negative1.dockerfile diff --git a/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/test/positive1.dockerfile b/assets/queries/dockerfile/using_unnamed_build_stages/test/positive1.dockerfile similarity index 100% rename from assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/test/positive1.dockerfile rename to assets/queries/dockerfile/using_unnamed_build_stages/test/positive1.dockerfile diff --git a/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/test/positive_expected_result.json b/assets/queries/dockerfile/using_unnamed_build_stages/test/positive_expected_result.json similarity index 54% rename from assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/test/positive_expected_result.json rename to assets/queries/dockerfile/using_unnamed_build_stages/test/positive_expected_result.json index 139bfda874c..a065d501187 100644 --- a/assets/queries/dockerfile/copy_from_without_from_alias_defined_previously/test/positive_expected_result.json +++ b/assets/queries/dockerfile/using_unnamed_build_stages/test/positive_expected_result.json @@ -1,6 +1,6 @@ [ { - "queryName": "COPY '--from' Without FROM Alias Defined Previously", + "queryName": "Using Unnamed Build Stages", "severity": "MEDIUM", "line": 10, "filename": "positive1.dockerfile" From ffc68a7b3a67d25fefe7eb85cd14927df13870c6 Mon Sep 17 00:00:00 2001 From: cxMiguelSilva Date: Mon, 26 Sep 2022 12:03:10 +0100 Subject: [PATCH 3/6] make appsec requested changes --- .../query.rego | 17 ++++++++++++++++- .../using_unnamed_build_stages/metadata.json | 2 +- .../test/positive_expected_result.json | 2 +- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/assets/queries/dockerfile/changing_default_shell_using_run_command/query.rego b/assets/queries/dockerfile/changing_default_shell_using_run_command/query.rego index a141507b89d..d217ce50692 100644 --- a/assets/queries/dockerfile/changing_default_shell_using_run_command/query.rego +++ b/assets/queries/dockerfile/changing_default_shell_using_run_command/query.rego @@ -2,11 +2,26 @@ package Cx import data.generic.common as common_lib +shell_possibilities := { + "/bin/bash", + "/bin/tcsh", + "/bin/ksh", + "/bin/csh", + "/bin/dash", + "etc/shells", + "/bin/zsh", + "/bin/fish", + "/bin/tmux", + "/bin/rbash", + "/bin/sh", + "/usr/bin/zsh", +} + CxPolicy[result] { resource := input.document[i].command[name][_] resource.Cmd == "run" value := resource.Value - shell_possibilities := {"/bin/bash", "/bin/tcsh", " /bin/ksh", "/bin/csh", "/bin/dash" , "etc/shells", "/zsh", "/bin/fish", "/bin/tmux", "/bin/rbash"} + contains(value[v], shell_possibilities[p]) run_values := split(value[v], " ") command := run_values[0] diff --git a/assets/queries/dockerfile/using_unnamed_build_stages/metadata.json b/assets/queries/dockerfile/using_unnamed_build_stages/metadata.json index 4d778e4a998..a86ba7ad0e7 100644 --- a/assets/queries/dockerfile/using_unnamed_build_stages/metadata.json +++ b/assets/queries/dockerfile/using_unnamed_build_stages/metadata.json @@ -1,7 +1,7 @@ { "id": "68a51e22-ae5a-4d48-8e87-b01a323605c9", "queryName": "Using Unnamed Build Stages", - "severity": "MEDIUM", + "severity": "LOW", "category": "Build Process", "descriptionText": " This query is used to ensure that build stages are named. This way even if the Dockerfile is re-ordered, the COPY instruction doesn’t break.", "descriptionUrl": "https://docs.docker.com/develop/develop-images/multistage-build/", diff --git a/assets/queries/dockerfile/using_unnamed_build_stages/test/positive_expected_result.json b/assets/queries/dockerfile/using_unnamed_build_stages/test/positive_expected_result.json index a065d501187..d0e9eb1f3db 100644 --- a/assets/queries/dockerfile/using_unnamed_build_stages/test/positive_expected_result.json +++ b/assets/queries/dockerfile/using_unnamed_build_stages/test/positive_expected_result.json @@ -1,7 +1,7 @@ [ { "queryName": "Using Unnamed Build Stages", - "severity": "MEDIUM", + "severity": "LOW", "line": 10, "filename": "positive1.dockerfile" } From 2402f5156ca865ad11ebfbbb9e1113a89408684f Mon Sep 17 00:00:00 2001 From: cxMiguelSilva Date: Tue, 27 Sep 2022 11:13:10 +0100 Subject: [PATCH 4/6] update run_command_cd_instead_of_workdir --- .../dockerfile/run_command_cd_instead_of_workdir/query.rego | 6 ++++-- .../test/negative2.dockerfile | 6 ++++++ .../test/positive.dockerfile | 2 +- 3 files changed, 11 insertions(+), 3 deletions(-) create mode 100644 assets/queries/dockerfile/run_command_cd_instead_of_workdir/test/negative2.dockerfile diff --git a/assets/queries/dockerfile/run_command_cd_instead_of_workdir/query.rego b/assets/queries/dockerfile/run_command_cd_instead_of_workdir/query.rego index 6f6d560fb9c..0721ae81965 100644 --- a/assets/queries/dockerfile/run_command_cd_instead_of_workdir/query.rego +++ b/assets/queries/dockerfile/run_command_cd_instead_of_workdir/query.rego @@ -3,12 +3,14 @@ package Cx CxPolicy[result] { resource := input.document[i].command[name][_] resource.Cmd == "run" - contains(resource.Value[_], "cd ") + run_command := resource.Value[_] + values := split(run_command, " ") + trim_space(values[0]) == "cd" result := { "documentId": input.document[i].id, "searchKey": sprintf("FROM={{%s}}.RUN={{%s}}", [name, resource.Value[0]]), - "issueType": "IncorrectValue", #"MissingAttribute" / "RedundantAttribute" + "issueType": "IncorrectValue", "keyExpectedValue": "Using WORKDIR to change directory", "keyActualValue": sprintf("RUN %s'", [resource.Value[0]]), } diff --git a/assets/queries/dockerfile/run_command_cd_instead_of_workdir/test/negative2.dockerfile b/assets/queries/dockerfile/run_command_cd_instead_of_workdir/test/negative2.dockerfile new file mode 100644 index 00000000000..55a411011f1 --- /dev/null +++ b/assets/queries/dockerfile/run_command_cd_instead_of_workdir/test/negative2.dockerfile @@ -0,0 +1,6 @@ +FROM nginx +ENV AUTHOR=Docker +RUN apk add --no-cache python \ + cd /usr/share/nginx/html +COPY Hello_docker.html /usr/share/nginx/html +CMD cd /usr/share/nginx/html && sed -e s/Docker/"$AUTHOR"/ Hello_docker.html > index.html ; nginx -g 'daemon off;' diff --git a/assets/queries/dockerfile/run_command_cd_instead_of_workdir/test/positive.dockerfile b/assets/queries/dockerfile/run_command_cd_instead_of_workdir/test/positive.dockerfile index 76717f5f418..c389c424c34 100644 --- a/assets/queries/dockerfile/run_command_cd_instead_of_workdir/test/positive.dockerfile +++ b/assets/queries/dockerfile/run_command_cd_instead_of_workdir/test/positive.dockerfile @@ -2,4 +2,4 @@ FROM nginx ENV AUTHOR=Docker RUN cd /usr/share/nginx/html COPY Hello_docker.html /usr/share/nginx/html -CMD cd /usr/share/nginx/html && sed -e s/Docker/"$AUTHOR"/ Hello_docker.html > index.html ; nginx -g 'daemon off;' \ No newline at end of file +CMD cd /usr/share/nginx/html && sed -e s/Docker/"$AUTHOR"/ Hello_docker.html > index.html ; nginx -g 'daemon off;' From cb7d6f83317c7f01612b85fc935cf8ab7b7be8c5 Mon Sep 17 00:00:00 2001 From: cxMiguelSilva Date: Tue, 27 Sep 2022 12:09:29 +0100 Subject: [PATCH 5/6] update query --- .../metadata.json | 2 +- .../run_command_cd_instead_of_workdir/query.rego | 12 +++++++++++- .../test/negative2.dockerfile | 3 +-- .../test/positive.dockerfile | 14 +++++++++++++- .../test/positive_expected_result.json | 15 ++++++++++++++- 5 files changed, 40 insertions(+), 6 deletions(-) diff --git a/assets/queries/dockerfile/run_command_cd_instead_of_workdir/metadata.json b/assets/queries/dockerfile/run_command_cd_instead_of_workdir/metadata.json index 9b89cccffc1..3e8359c12b4 100644 --- a/assets/queries/dockerfile/run_command_cd_instead_of_workdir/metadata.json +++ b/assets/queries/dockerfile/run_command_cd_instead_of_workdir/metadata.json @@ -3,7 +3,7 @@ "queryName": "RUN Instruction Using 'cd' Instead of WORKDIR", "severity": "MEDIUM", "category": "Build Process", - "descriptionText": "The user can utilize WORKDIR or the full path instead of proliferating instructions like RUN cd \u2026 && do-something, which are hard to read, troubleshoot, and maintain. Although using the full path can also make the code harder to read sometimes, it is preferable.", + "descriptionText": "When using RUN command 'cd' should be only be used for full path for relative path use WORKDIR command instead", "descriptionUrl": "https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#workdir", "platform": "Dockerfile", "descriptionID": "edd9f7d3" diff --git a/assets/queries/dockerfile/run_command_cd_instead_of_workdir/query.rego b/assets/queries/dockerfile/run_command_cd_instead_of_workdir/query.rego index 0721ae81965..9a75bbd392b 100644 --- a/assets/queries/dockerfile/run_command_cd_instead_of_workdir/query.rego +++ b/assets/queries/dockerfile/run_command_cd_instead_of_workdir/query.rego @@ -5,7 +5,10 @@ CxPolicy[result] { resource.Cmd == "run" run_command := resource.Value[_] values := split(run_command, " ") - trim_space(values[0]) == "cd" + trim_space(values[index]) == "cd" + path := trim_space(values[index+1]) + not is_full_path(path) + result := { "documentId": input.document[i].id, @@ -15,3 +18,10 @@ CxPolicy[result] { "keyActualValue": sprintf("RUN %s'", [resource.Value[0]]), } } + +is_full_path(path){ + regex.match("^[a-zA-Z]:[\\\/]", path) +}else { + startswith( path,"/") + not contains(path, "/.") +} diff --git a/assets/queries/dockerfile/run_command_cd_instead_of_workdir/test/negative2.dockerfile b/assets/queries/dockerfile/run_command_cd_instead_of_workdir/test/negative2.dockerfile index 55a411011f1..c389c424c34 100644 --- a/assets/queries/dockerfile/run_command_cd_instead_of_workdir/test/negative2.dockerfile +++ b/assets/queries/dockerfile/run_command_cd_instead_of_workdir/test/negative2.dockerfile @@ -1,6 +1,5 @@ FROM nginx ENV AUTHOR=Docker -RUN apk add --no-cache python \ - cd /usr/share/nginx/html +RUN cd /usr/share/nginx/html COPY Hello_docker.html /usr/share/nginx/html CMD cd /usr/share/nginx/html && sed -e s/Docker/"$AUTHOR"/ Hello_docker.html > index.html ; nginx -g 'daemon off;' diff --git a/assets/queries/dockerfile/run_command_cd_instead_of_workdir/test/positive.dockerfile b/assets/queries/dockerfile/run_command_cd_instead_of_workdir/test/positive.dockerfile index c389c424c34..2a34f9c9b19 100644 --- a/assets/queries/dockerfile/run_command_cd_instead_of_workdir/test/positive.dockerfile +++ b/assets/queries/dockerfile/run_command_cd_instead_of_workdir/test/positive.dockerfile @@ -1,5 +1,17 @@ FROM nginx ENV AUTHOR=Docker -RUN cd /usr/share/nginx/html +RUN cd /../share/nginx/html +COPY Hello_docker.html /usr/share/nginx/html +CMD cd /usr/share/nginx/html && sed -e s/Docker/"$AUTHOR"/ Hello_docker.html > index.html ; nginx -g 'daemon off;' + +FROM nginx +ENV AUTHOR=Docker +RUN cd ../share/nginx/html +COPY Hello_docker.html /usr/share/nginx/html +CMD cd /usr/share/nginx/html && sed -e s/Docker/"$AUTHOR"/ Hello_docker.html > index.html ; nginx -g 'daemon off;' + +FROM nginx +ENV AUTHOR=Docker +RUN cd /usr/../share/nginx/html COPY Hello_docker.html /usr/share/nginx/html CMD cd /usr/share/nginx/html && sed -e s/Docker/"$AUTHOR"/ Hello_docker.html > index.html ; nginx -g 'daemon off;' diff --git a/assets/queries/dockerfile/run_command_cd_instead_of_workdir/test/positive_expected_result.json b/assets/queries/dockerfile/run_command_cd_instead_of_workdir/test/positive_expected_result.json index 758167166d5..81f43312460 100644 --- a/assets/queries/dockerfile/run_command_cd_instead_of_workdir/test/positive_expected_result.json +++ b/assets/queries/dockerfile/run_command_cd_instead_of_workdir/test/positive_expected_result.json @@ -2,6 +2,19 @@ { "queryName": "RUN Instruction Using 'cd' Instead of WORKDIR", "severity": "MEDIUM", - "line": 3 + "line": 3, + "fileName": "positive.dockerfile" + }, + { + "queryName": "RUN Instruction Using 'cd' Instead of WORKDIR", + "severity": "MEDIUM", + "line": 9, + "fileName": "positive.dockerfile" + }, + { + "queryName": "RUN Instruction Using 'cd' Instead of WORKDIR", + "severity": "MEDIUM", + "line": 15, + "fileName": "positive.dockerfile" } ] From a028e4d4ce924696b1e41060b271aae8657d6bfa Mon Sep 17 00:00:00 2001 From: cxMiguelSilva Date: Wed, 28 Sep 2022 10:48:03 +0100 Subject: [PATCH 6/6] correct typo --- .../dockerfile/run_command_cd_instead_of_workdir/metadata.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/queries/dockerfile/run_command_cd_instead_of_workdir/metadata.json b/assets/queries/dockerfile/run_command_cd_instead_of_workdir/metadata.json index 3e8359c12b4..20ad068fe88 100644 --- a/assets/queries/dockerfile/run_command_cd_instead_of_workdir/metadata.json +++ b/assets/queries/dockerfile/run_command_cd_instead_of_workdir/metadata.json @@ -3,7 +3,7 @@ "queryName": "RUN Instruction Using 'cd' Instead of WORKDIR", "severity": "MEDIUM", "category": "Build Process", - "descriptionText": "When using RUN command 'cd' should be only be used for full path for relative path use WORKDIR command instead", + "descriptionText": "When using RUN command 'cd' should only be used for full path. For relative path make use of WORKDIR command instead.", "descriptionUrl": "https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#workdir", "platform": "Dockerfile", "descriptionID": "edd9f7d3"