diff --git a/assets/queries/dockerfile/last_user_is_root/metadata.json b/assets/queries/dockerfile/last_user_is_root/metadata.json index ed4a7e9198d..9826eaefbf7 100644 --- a/assets/queries/dockerfile/last_user_is_root/metadata.json +++ b/assets/queries/dockerfile/last_user_is_root/metadata.json @@ -3,10 +3,11 @@ "queryName": "Last User Is 'root'", "severity": "HIGH", "category": "Best Practices", - "descriptionText": "Leaving the last user as root can cause security risks. Change to another user after running the commands the need privileges", + "descriptionText": "Leaving the last user as root can cause security risks. Change to another user after running the commands that need privileges", "descriptionUrl": "https://docs.docker.com/engine/reference/builder/#user", "platform": "Dockerfile", "descriptionID": "f445bd25", + "cloudProvider": "common", "cwe": "250", "oldSeverity": "MEDIUM" } \ No newline at end of file diff --git a/docs/queries/dockerfile-queries/67fd0c4a-68cf-46d7-8c41-bc9fba7e40ae.md b/docs/queries/dockerfile-queries/67fd0c4a-68cf-46d7-8c41-bc9fba7e40ae.md index eee035c2489..4109c2b551b 100644 --- a/docs/queries/dockerfile-queries/67fd0c4a-68cf-46d7-8c41-bc9fba7e40ae.md +++ b/docs/queries/dockerfile-queries/67fd0c4a-68cf-46d7-8c41-bc9fba7e40ae.md @@ -24,7 +24,7 @@ hide: - **URL:** [Github](https://github.com/Checkmarx/kics/tree/master/assets/queries/dockerfile/last_user_is_root) ### Description -Leaving the last user as root can cause security risks. Change to another user after running the commands the need privileges
+Leaving the last user as root can cause security risks. Change to another user after running the commands that need privileges
[Documentation](https://docs.docker.com/engine/reference/builder/#user) ### Code samples diff --git a/e2e/fixtures/schemas/result.json b/e2e/fixtures/schemas/result.json index 80a3cdbd1c5..4d61250ea22 100644 --- a/e2e/fixtures/schemas/result.json +++ b/e2e/fixtures/schemas/result.json @@ -81,7 +81,7 @@ }, "query_id": { "type": "string", - "pattern": "^[a-f0-9]{8}-[a-f0-9]{4}-4{1}[a-f0-9]{3}-[89ab]{1}[a-f0-9]{3}-[a-f0-9]{12}$" + "pattern": "^(?[a-f0-9]{8}-[a-f0-9]{4}-4{1}[a-f0-9]{3}-[89ab]{1}[a-f0-9]{3}-[a-f0-9]{12})|(?(t:|p:|a:)(\\d{1,20}))$" }, "query_url": { "type": "string", diff --git a/e2e/testcases/e2e-cli-098_include_ids_with_new_queryid_validation.go b/e2e/testcases/e2e-cli-098_include_ids_with_new_queryid_validation.go new file mode 100644 index 00000000000..dccdf43fe8b --- /dev/null +++ b/e2e/testcases/e2e-cli-098_include_ids_with_new_queryid_validation.go @@ -0,0 +1,31 @@ +package testcases + +// E2E-CLI-098 +// should perform the scan successfully and return exit code 50 +// this test sample contains a different query_id +// that is not a UUID, but contains a prefix ('t:', 'p:', or 'a:') + uint64 +func init() { //nolint + testSample := TestCase{ + Name: "should perform a valid scan and return one HIGH result [E2E-CLI-098]", + Args: args{ + Args: []cmdArgs{ + []string{"scan", "-o", "/path/e2e/output", + "--output-name", "E2E_CLI_098_RESULT", + "-q", "\"/path/test/fixtures/new_queryid_validation\"", + "-p", "\"/path/test/fixtures/new_queryid_validation/Dockerfile\"", + // QueryID 'a:123' does not exist, however, since the first one does, it should perform the scan successfully + "-i", "t:8820143918834007824,a:123", + }, + }, + ExpectedResult: []ResultsValidation{ + { + ResultsFile: "E2E_CLI_098_RESULT", + ResultsFormats: []string{"json"}, + }, + }, + }, + WantStatus: []int{50}, + } + + Tests = append(Tests, testSample) +} diff --git a/internal/console/flags/validate.go b/internal/console/flags/validate.go index 275464f6426..765f3900489 100644 --- a/internal/console/flags/validate.go +++ b/internal/console/flags/validate.go @@ -14,8 +14,15 @@ var flagValidationFuncs = flagValidationFuncsMap{ } func isQueryID(id string) bool { - re := regexp.MustCompile(`^[0-9a-fA-F]{8}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{12}$`) - return re.MatchString(id) + uuidRegex := regexp.MustCompile(`^[0-9a-fA-F]{8}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{12}$`) + isQueryID := uuidRegex.MatchString(id) + if !isQueryID { + // (t:|p:|a:) matches strings starting with 't:', 'p:', or 'a:' + // (\d{1,20}) ensures the numeric part has 1 to 20 digits (uint64 validation) + cxoneRegex := regexp.MustCompile(`^(t:|p:|a:)(\d{1,20})$`) + isQueryID = cxoneRegex.MatchString(id) + } + return isQueryID } func convertSliceToDummyMap(slice []string) map[string]string { diff --git a/internal/console/flags/validate_test.go b/internal/console/flags/validate_test.go index 1b8f3934b71..8b5d75d7ea1 100644 --- a/internal/console/flags/validate_test.go +++ b/internal/console/flags/validate_test.go @@ -22,6 +22,26 @@ func TestFlags_isQueryID(t *testing.T) { id: "test", expected: false, }, + { + name: "for prefix 't:' should return that query id is valid", + id: "t:12345678901234567890", + expected: true, + }, + { + name: "for prefix 'p:' should return that query id is valid", + id: "p:8820143918834007824", + expected: true, + }, + { + name: "for prefix 'a:' should return that query id is valid", + id: "a:8820143918834007824", + expected: true, + }, + { + name: "should return that query id is invalid because uint exceeds 20 length", + id: "t:123456789012345678901", + expected: false, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { diff --git a/test/fixtures/new_queryid_validation/Dockerfile b/test/fixtures/new_queryid_validation/Dockerfile new file mode 100644 index 00000000000..09c9ccaca40 --- /dev/null +++ b/test/fixtures/new_queryid_validation/Dockerfile @@ -0,0 +1,3 @@ +FROM alpine:2.6 +USER guest +RUN npm install \ No newline at end of file diff --git a/test/fixtures/new_queryid_validation/metadata.json b/test/fixtures/new_queryid_validation/metadata.json new file mode 100644 index 00000000000..622648602b0 --- /dev/null +++ b/test/fixtures/new_queryid_validation/metadata.json @@ -0,0 +1,12 @@ +{ + "id": "t:8820143918834007824", + "queryName": "Last User Is 'guest'", + "severity": "HIGH", + "category": "Best Practices", + "descriptionText": "Leaving the last user as guest can cause security risks. Change to another user after running the commands that need privileges", + "descriptionUrl": "https://docs.docker.com/engine/reference/builder/#user", + "platform": "Dockerfile", + "descriptionID": "f445bd25", + "cwe": "250", + "oldSeverity": "MEDIUM" +} \ No newline at end of file diff --git a/test/fixtures/new_queryid_validation/query.rego b/test/fixtures/new_queryid_validation/query.rego new file mode 100644 index 00000000000..36e6b8bb9a8 --- /dev/null +++ b/test/fixtures/new_queryid_validation/query.rego @@ -0,0 +1,19 @@ +package Cx + +import data.generic.dockerfile as dockerLib + +CxPolicy[result] { + resource := input.document[i].command[name] + dockerLib.check_multi_stage(name, input.document[i].command) + + userCmd := [x | resource[j].Cmd == "user"; x := resource[j]] + userCmd[minus(count(userCmd), 1)].Value[0] == "guest" + + result := { + "documentId": input.document[i].id, + "searchKey": sprintf("FROM={{%s}}.{{%s}}", [name, userCmd[minus(count(userCmd), 1)].Original]), + "issueType": "IncorrectValue", + "keyExpectedValue": "Last User shouldn't be guest", + "keyActualValue": "Last User is guest", + } +}