From 2bf7a18d03ca81828e9787bc6f2e078cada9b3f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Reigota?= Date: Mon, 5 Apr 2021 14:50:25 +0100 Subject: [PATCH] Refactored Vulnerability Builder (#2531) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Rogério Peixoto --- go.sum | 2 - internal/console/helpers/helpers.go | 8 +- internal/constants/constants.go | 5 + pkg/detector/default_detect.go | 56 ++ pkg/detector/default_detect_test.go | 156 +++++ pkg/detector/detector.go | 49 ++ pkg/detector/detector_test.go | 151 +++++ pkg/detector/docker/docker_detect.go | 85 +++ pkg/detector/docker/docker_detect_test.go | 140 +++++ pkg/detector/helm/helm_detect.go | 187 ++++++ pkg/detector/helm/helm_detect_test.go | 193 ++++++ pkg/detector/helper.go | 222 +++++++ pkg/detector/helper_test.go | 332 ++++++++++ pkg/engine/inspector.go | 14 +- pkg/engine/inspector_test.go | 8 + pkg/engine/vulnerability_builder.go | 487 +-------------- pkg/engine/vulnerability_builder_test.go | 722 +--------------------- pkg/kics/resolver_sink.go | 52 ++ pkg/kics/service.go | 82 +-- pkg/kics/service_test.go | 14 +- pkg/kics/sink.go | 47 ++ pkg/model/model.go | 57 +- pkg/model/summary.go | 20 +- pkg/report/template/html/report.tmpl | 11 +- pkg/resolver/helm/resolver.go | 2 +- test/queries_content_test.go | 3 +- 26 files changed, 1780 insertions(+), 1325 deletions(-) create mode 100644 pkg/detector/default_detect.go create mode 100644 pkg/detector/default_detect_test.go create mode 100644 pkg/detector/detector.go create mode 100644 pkg/detector/detector_test.go create mode 100644 pkg/detector/docker/docker_detect.go create mode 100644 pkg/detector/docker/docker_detect_test.go create mode 100644 pkg/detector/helm/helm_detect.go create mode 100644 pkg/detector/helm/helm_detect_test.go create mode 100644 pkg/detector/helper.go create mode 100644 pkg/detector/helper_test.go create mode 100644 pkg/kics/resolver_sink.go create mode 100644 pkg/kics/sink.go diff --git a/go.sum b/go.sum index b7c8f784d19..4bc7e95d49d 100644 --- a/go.sum +++ b/go.sum @@ -1319,7 +1319,6 @@ golang.org/x/mod v0.1.0/go.mod h1:0QHyrYULN0/3qlju5TqG8bIK38QM8yzMo5ekMj3DlcY= golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg= golang.org/x/mod v0.1.1-0.20191107180719-034126e5016b/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= -golang.org/x/mod v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/net v0.0.0-20170114055629-f2499483f923/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -1566,7 +1565,6 @@ golang.org/x/tools v0.0.0-20200505023115-26f46d2f7ef8/go.mod h1:EkVYQZoAsY45+roY golang.org/x/tools v0.0.0-20200512131952-2bc93b1c0c88/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20200616133436-c1934b75d054/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20200804011535-6c149bb5ef0d/go.mod h1:njjCfa9FT2d7l9Bc6FUM5FLjQPp3cFF28FI3qnDFljA= -golang.org/x/tools v0.1.0 h1:po9/4sTYwZU9lPhi1tOrb4hCv3qrhiQ77LZfGa2OjwY= golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/internal/console/helpers/helpers.go b/internal/console/helpers/helpers.go index 00ad6364698..401ae8e2d7e 100644 --- a/internal/console/helpers/helpers.go +++ b/internal/console/helpers/helpers.go @@ -174,11 +174,11 @@ func printFiles(query *model.VulnerableQuery, printer *Printer) { query.Files[fileIdx].FileName, printer.Success.Sprint(query.Files[fileIdx].Line)) if !printer.minimal { fmt.Println() - for lineIdx, line := range query.Files[fileIdx].VulnLines.Lines { - if query.Files[fileIdx].VulnLines.Positions[lineIdx] == query.Files[fileIdx].Line { - printer.Line.Printf("\t\t%03d: %s\n", query.Files[fileIdx].VulnLines.Positions[lineIdx], line) + for _, line := range query.Files[fileIdx].VulnLines { + if line.Position == query.Files[fileIdx].Line { + printer.Line.Printf("\t\t%03d: %s\n", line.Position, line.Line) } else { - fmt.Printf("\t\t%03d: %s\n", query.Files[fileIdx].VulnLines.Positions[lineIdx], line) + fmt.Printf("\t\t%03d: %s\n", line.Position, line.Line) } } fmt.Print("\n\n") diff --git a/internal/constants/constants.go b/internal/constants/constants.go index bd5df2a972a..045914250ee 100644 --- a/internal/constants/constants.go +++ b/internal/constants/constants.go @@ -1,5 +1,7 @@ package constants +import "math" + // Version - current KICS version var Version = "dev" @@ -20,3 +22,6 @@ const MinimumPreviewLines = 1 // MaximumPreviewLines - default maximum preview lines number const MaximumPreviewLines = 30 + +// MaxInteger - max possible integer in golang +const MaxInteger = math.MaxInt64 diff --git a/pkg/detector/default_detect.go b/pkg/detector/default_detect.go new file mode 100644 index 00000000000..0356ec9220a --- /dev/null +++ b/pkg/detector/default_detect.go @@ -0,0 +1,56 @@ +package detector + +import ( + "strconv" + "strings" + + "github.com/Checkmarx/kics/pkg/model" + "github.com/rs/zerolog" +) + +const ( + undetectedVulnerabilityLine = -1 +) + +type defaultDetectLine struct { +} + +// DetectLine searches vulnerability line if kindDetectLine is not in detectors +func (d defaultDetectLine) DetectLine(file *model.FileMetadata, searchKey string, + logWithFields *zerolog.Logger, outputLines int) model.VulnerabilityLines { + text := strings.ReplaceAll(file.OriginalData, "\r", "") + lines := strings.Split(text, "\n") + foundAtLeastOne := false + currentLine := 0 + isBreak := false + var extractedString [][]string + extractedString = GetBracketValues(searchKey, extractedString, "") + sanitizedSubstring := searchKey + for idx, str := range extractedString { + sanitizedSubstring = strings.Replace(sanitizedSubstring, str[0], `{{`+strconv.Itoa(idx)+`}}`, -1) + } + + for _, key := range strings.Split(sanitizedSubstring, ".") { + substr1, substr2 := GenerateSubstrings(key, extractedString) + + foundAtLeastOne, currentLine, isBreak = DetectCurrentLine(lines, substr1, substr2, currentLine, foundAtLeastOne) + + if isBreak { + break + } + } + + if foundAtLeastOne { + return model.VulnerabilityLines{ + Line: currentLine + 1, + VulnLines: GetAdjacentVulnLines(currentLine, outputLines, lines), + } + } + + logWithFields.Warn().Msgf("Failed to detect line, query response %s", searchKey) + + return model.VulnerabilityLines{ + Line: undetectedVulnerabilityLine, + VulnLines: []model.CodeLine{}, + } +} diff --git a/pkg/detector/default_detect_test.go b/pkg/detector/default_detect_test.go new file mode 100644 index 00000000000..1f4d39221c5 --- /dev/null +++ b/pkg/detector/default_detect_test.go @@ -0,0 +1,156 @@ +package detector + +import ( + "reflect" + "testing" + + "github.com/Checkmarx/kics/pkg/model" + "github.com/Checkmarx/kics/test" + "github.com/rs/zerolog" + "github.com/stretchr/testify/require" +) + +// Test_detectLine tests the functions [detectLine()] and all the methods called by them +func Test_detectLine(t *testing.T) { //nolint + type args struct { + file *model.FileMetadata + searchKey string + } + type feilds struct { + outputLines int + } + tests := []struct { + name string + args args + feilds feilds + want model.VulnerabilityLines + }{ + { + name: "detect_line", + args: args{ + file: &model.FileMetadata{ + ScanID: "scanID", + ID: "Test", + Kind: model.KindTerraform, + OriginalData: `resource "aws_s3_bucket" "b" { + bucket = "my-tf-test-bucket" + acl = "authenticated-read" + + tags = { + Name = "My bucket" + Environment = "Dev" + } + } + `, + }, + searchKey: "aws_s3_bucket[b].acl", + }, + feilds: feilds{ + outputLines: 3, + }, + want: model.VulnerabilityLines{ + Line: 3, + VulnLines: []model.CodeLine{ + { + Position: 2, + Line: ` bucket = "my-tf-test-bucket"`, + }, + { + Position: 3, + Line: ` acl = "authenticated-read"`, + }, + { + Position: 4, + Line: "", + }, + }, + LineWithVulnerabilty: "", + }, + }, + { + name: "detect_line_with_curly_brackets", + args: args{ + file: &model.FileMetadata{ + ScanID: "scanID", + ID: "Test", + Kind: model.KindTerraform, + OriginalData: `resource "aws_s3_bucket" "b" { + bucket = "my-tf-test-bucket" + acl = "authenticated-read" + + tags = { + Name = "My bucket" + Environment = "Dev.123" + Environment = "test" + } + } + `, + }, + searchKey: "aws_s3_bucket[b].Environment={{Dev.123}}", + }, + feilds: feilds{ + outputLines: 3, + }, + want: model.VulnerabilityLines{ + Line: 7, + VulnLines: []model.CodeLine{ + { + Position: 6, + Line: ` Name = "My bucket"`, + }, + { + Position: 7, + Line: ` Environment = "Dev.123"`, + }, + { + Position: 8, + Line: ` Environment = "test"`, + }, + }, + LineWithVulnerabilty: "", + }, + }, + { + name: "detect_line_error", + args: args{ + file: &model.FileMetadata{ + ScanID: "scanID", + ID: "Test", + Kind: model.KindTerraform, + OriginalData: `resource "aws_s3_bucket" "b" { + bucket = "my-tf-test-bucket" + acl = "authenticated-read" + + tags = { + Name = "My bucket" + Environment = "Dev.123" + Environment = "test" + } + } + `, + }, + searchKey: "testing.error", + }, + feilds: feilds{ + outputLines: 3, + }, + want: model.VulnerabilityLines{ + Line: -1, + VulnLines: []model.CodeLine{}, + }, + }, + } + for _, tt := range tests { + detector := NewDetectLine(tt.feilds.outputLines) + t.Run(tt.name, func(t *testing.T) { + got := detector.defaultDetector.DetectLine(tt.args.file, tt.args.searchKey, &zerolog.Logger{}, 3) + gotStrVulnerabilities, err := test.StringifyStruct(got) + require.Nil(t, err) + wantStrVulnerabilities, err := test.StringifyStruct(tt.want) + require.Nil(t, err) + if !reflect.DeepEqual(gotStrVulnerabilities, wantStrVulnerabilities) { + t.Errorf("detectLine() = %v, want %v", gotStrVulnerabilities, wantStrVulnerabilities) + } + }) + } +} diff --git a/pkg/detector/detector.go b/pkg/detector/detector.go new file mode 100644 index 00000000000..6e9b75c50e7 --- /dev/null +++ b/pkg/detector/detector.go @@ -0,0 +1,49 @@ +package detector + +import ( + "github.com/Checkmarx/kics/pkg/model" + "github.com/rs/zerolog" +) + +type kindDetectLine interface { + DetectLine(file *model.FileMetadata, searchKey string, + logWithFields *zerolog.Logger, outputLines int) model.VulnerabilityLines +} + +// DetectLine is a struct that associates a kindDetectLine to its FileKind +type DetectLine struct { + detectors map[model.FileKind]kindDetectLine + outputLines int + logWithFields *zerolog.Logger + defaultDetector kindDetectLine +} + +// NewDetectLine creates a new DetectLine's reference +func NewDetectLine(outputLines int) *DetectLine { + return &DetectLine{ + detectors: make(map[model.FileKind]kindDetectLine), + logWithFields: &zerolog.Logger{}, + outputLines: outputLines, + defaultDetector: defaultDetectLine{}, + } +} + +// SetupLogs will change the logger feild to be used in kindDetectLine DetectLine method +func (d *DetectLine) SetupLogs(logger *zerolog.Logger) { + d.logWithFields = logger +} + +// Add adds a new kindDetectLine to the caller and returns it +func (d *DetectLine) Add(detector kindDetectLine, kind model.FileKind) *DetectLine { + d.detectors[kind] = detector + return d +} + +// DetectLine will use the correct kindDetectLine according to the files kind +// if file kind is not in detectors default detect line is called +func (d *DetectLine) DetectLine(file *model.FileMetadata, searchKey string) model.VulnerabilityLines { + if det, ok := d.detectors[file.Kind]; ok { + return det.DetectLine(file, searchKey, d.logWithFields, d.outputLines) + } + return d.defaultDetector.DetectLine(file, searchKey, d.logWithFields, d.outputLines) +} diff --git a/pkg/detector/detector_test.go b/pkg/detector/detector_test.go new file mode 100644 index 00000000000..e4834606a49 --- /dev/null +++ b/pkg/detector/detector_test.go @@ -0,0 +1,151 @@ +package detector + +import ( + "reflect" + "testing" + + "github.com/Checkmarx/kics/pkg/model" + "github.com/rs/zerolog" + "github.com/rs/zerolog/log" +) + +type mockkindDetectLine struct { +} + +type mockDefaultDetector struct { +} + +func (m mockkindDetectLine) DetectLine(file *model.FileMetadata, searchKey string, + logWithFields *zerolog.Logger, outputLines int) model.VulnerabilityLines { + return model.VulnerabilityLines{ + Line: 1, + } +} + +func (m mockDefaultDetector) DetectLine(file *model.FileMetadata, searchKey string, + logWithFields *zerolog.Logger, outputLines int) model.VulnerabilityLines { + return model.VulnerabilityLines{ + Line: 5, + } +} + +func TestDetector_Add(t *testing.T) { + var mock mockkindDetectLine + det := initDetector() + type args struct { + kindDetector kindDetectLine + fileKind model.FileKind + } + tests := []struct { + name string + args args + }{ + { + name: "test_add", + args: args{ + kindDetector: mock, + fileKind: model.KindDOCKER, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + det = det.Add(tt.args.kindDetector, tt.args.fileKind) + got, ok := det.detectors[tt.args.fileKind] + if !ok { + t.Errorf("Add(), mockKindDetectLine is not in detectors") + } + if !reflect.DeepEqual(got, mock) { + t.Errorf("Add() = %v, want = %v", got, mock) + } + }) + } +} + +func TestDetector_SetupLogs(t *testing.T) { + det := initDetector() + type args struct { + log zerolog.Logger + } + tests := []struct { + name string + args args + }{ + { + name: "test_setup_logs", + args: args{ + log: log.With(). + Str("scanID", "Test"). + Str("fileName", "Test_file_name"). + Str("queryName", "Test_Query_name"). + Logger(), + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + det.SetupLogs(&tt.args.log) + got := det.logWithFields + if !reflect.DeepEqual(*got, tt.args.log) { + t.Errorf("SetupLogs() = %v, want = %v", got, tt.args.log) + } + }) + } +} + +func TestDetector_DetectLine(t *testing.T) { + var mock mockkindDetectLine + var defaultmock mockDefaultDetector + det := initDetector().Add(mock, model.KindCOMMON) + det.defaultDetector = defaultmock + + type args struct { + file *model.FileMetadata + searchKey string + } + tests := []struct { + name string + args args + want model.VulnerabilityLines + }{ + { + name: "test_kind_detect_line", + args: args{ + file: &model.FileMetadata{ + Kind: model.KindCOMMON, + }, + searchKey: "", + }, + want: model.VulnerabilityLines{ + Line: 1, + }, + }, + { + name: "test_default_detect_line", + args: args{ + file: &model.FileMetadata{ + Kind: model.KindTerraform, + }, + searchKey: "", + }, + want: model.VulnerabilityLines{ + Line: 5, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := det.DetectLine(tt.args.file, tt.args.searchKey) + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("DetectLine() = %v, want = %v", got, tt.want) + } + }) + } +} + +func initDetector() *DetectLine { + return NewDetectLine(3) +} diff --git a/pkg/detector/docker/docker_detect.go b/pkg/detector/docker/docker_detect.go new file mode 100644 index 00000000000..9d94d481f30 --- /dev/null +++ b/pkg/detector/docker/docker_detect.go @@ -0,0 +1,85 @@ +package docker + +import ( + "regexp" + "strconv" + "strings" + + "github.com/Checkmarx/kics/pkg/detector" + "github.com/Checkmarx/kics/pkg/model" + "github.com/rs/zerolog" +) + +// DetectKindLine defines a kindDetectLine type +type DetectKindLine struct { +} + +const ( + undetectedVulnerabilityLine = -1 +) + +var ( + nameRegexDockerFileML = regexp.MustCompile(`.+\s+\\$`) +) + +// DetectLine searches vulnerability line in docker files +func (d DetectKindLine) DetectLine(file *model.FileMetadata, searchKey string, + logWithFields *zerolog.Logger, outputLines int) model.VulnerabilityLines { + text := strings.ReplaceAll(file.OriginalData, "\r", "") + lines := prepareDockerFileLines(text) + isBreak := false + foundAtLeastOne := false + currentLine := 0 + var extractedString [][]string + extractedString = detector.GetBracketValues(searchKey, extractedString, "") + sKey := searchKey + for idx, str := range extractedString { + sKey = strings.Replace(sKey, str[0], `{{`+strconv.Itoa(idx)+`}}`, -1) + } + + for _, key := range strings.Split(sKey, ".") { + substr1, substr2 := detector.GenerateSubstrings(key, extractedString) + + foundAtLeastOne, currentLine, isBreak = detector.DetectCurrentLine(lines, substr1, substr2, + currentLine, foundAtLeastOne) + + if isBreak { + break + } + } + + if foundAtLeastOne { + return model.VulnerabilityLines{ + Line: currentLine + 1, + VulnLines: detector.GetAdjacentVulnLines(currentLine, outputLines, strings.Split(text, "\n")), + } + } + + logWithFields.Warn().Msgf("Failed to detect Docker line, query response %s", searchKey) + + return model.VulnerabilityLines{ + Line: undetectedVulnerabilityLine, + VulnLines: []model.CodeLine{}, + } +} + +func prepareDockerFileLines(text string) []string { + textSplit := strings.Split(text, "\n") + for idx, key := range textSplit { + textSplit[idx] = multiLineSpliter(textSplit, key, idx) + } + return textSplit +} + +func multiLineSpliter(textSplit []string, key string, idx int) string { + if nameRegexDockerFileML.MatchString(key) { + i := idx + 1 + for textSplit[i] == "" { + i++ + } + textSplit[idx] = strings.ReplaceAll(textSplit[idx], " \\", " "+textSplit[i]) + textSplit[i] = "" + textSplit[idx] = multiLineSpliter(textSplit, textSplit[idx], idx) + } + return textSplit[idx] +} diff --git a/pkg/detector/docker/docker_detect_test.go b/pkg/detector/docker/docker_detect_test.go new file mode 100644 index 00000000000..26a9b807e48 --- /dev/null +++ b/pkg/detector/docker/docker_detect_test.go @@ -0,0 +1,140 @@ +package docker + +import ( + "fmt" + "testing" + + "github.com/Checkmarx/kics/pkg/model" + "github.com/rs/zerolog" + "github.com/stretchr/testify/require" +) + +// TestDetectDockerLine tests the functions [DetectDockerLine()] and all the methods called by them +func TestDetectDockerLine(t *testing.T) { //nolint + testCases := []struct { + expected model.VulnerabilityLines + searchKey string + file *model.FileMetadata + }{ + { + expected: model.VulnerabilityLines{ + Line: 10, + VulnLines: []model.CodeLine{ + { + Position: 9, + Line: "RUN apk update", + }, + { + Position: 10, + Line: "RUN apk update && apk upgrade && apk add kubectl=1.20.0-r0 \\", + }, + { + Position: 11, + Line: "\t&& rm -rf /var/cache/apk/*", + }, + }, + }, + searchKey: "FROM={{alpine:3.9}}.RUN={{apk update && apk upgrade && apk add kubectl=1.20.0-r0 \u0026\u0026 rm -rf /var/cache/apk/*}}", + file: &model.FileMetadata{ + ScanID: "Test2", + ID: "Test2", + Kind: model.KindDOCKER, + OriginalData: `FROM alpine:3.7 +RUN apk update \ + && apk upgrade \ + && apk add kubectl=1.20.0-r0 \ + && rm -rf /var/cache/apk/* +ENTRYPOINT ["kubectl"] + +FROM alpine:3.9 +RUN apk update +RUN apk update && apk upgrade && apk add kubectl=1.20.0-r0 \ + && rm -rf /var/cache/apk/* +ENTRYPOINT ["kubectl"] +`, + }, + }, + { + expected: model.VulnerabilityLines{ + Line: 17, + VulnLines: []model.CodeLine{ + { + Position: 16, + Line: "ARG JAR_FILE", + }, + { + Position: 17, + Line: "ADD ${JAR_FILE} apps.jar", + }, + { + Position: 18, + Line: "", + }, + }, + }, + searchKey: "FROM=openjdk:11-jdk.{{ADD ${JAR_FILE} apps.jar}}", + file: &model.FileMetadata{ + ScanID: "Test3", + ID: "Test3", + Kind: model.KindDOCKER, + OriginalData: `FROM openjdk:10-jdk +VOLUME /tmp +ADD http://source.file/package.file.tar.gz /temp +RUN tar -xjf /temp/package.file.tar.gz \ + && make -C /tmp/package.file \ + && rm /tmp/ package.file.tar.gz +ARG JAR_FILE +ADD ${JAR_FILE} app.jar + +FROM openjdk:11-jdk +VOLUME /tmp +ADD http://source.file/package.file.tar.gz /temp +RUN tar -xjf /temp/package.file.tar.gz \ + && make -C /tmp/package.file \ + && rm /tmp/ package.file.tar.gz +ARG JAR_FILE +ADD ${JAR_FILE} apps.jar +`, + }, + }, + { + expected: model.VulnerabilityLines{ + Line: 6, + VulnLines: []model.CodeLine{ + { + Position: 5, + Line: ` && apk add kubectl=1.20.0-r0 \`, + }, + { + Position: 6, + Line: " && rm -rf /var/cache/apk/*", + }, + { + Position: 7, + Line: `ENTRYPOINT ["kubectl"]`, + }, + }, + }, + searchKey: "FROM={{alpine:3.7}}.ENTRYPOINT[kubectl]", + file: &model.FileMetadata{ + ScanID: "Test", + ID: "Test", + Kind: model.KindDOCKER, + OriginalData: `FROM alpine:3.7 +RUN apk update \ + && apk upgrade \ + && apk add kubectl=1.20.0-r0 \ + && rm -rf /var/cache/apk/* +ENTRYPOINT ["kubectl"]`, + }, + }, + } + + for i, testCase := range testCases { + detector := DetectKindLine{} + t.Run(fmt.Sprintf("detectDockerLine-%d", i), func(t *testing.T) { + v := detector.DetectLine(testCase.file, testCase.searchKey, &zerolog.Logger{}, 3) + require.Equal(t, testCase.expected, v) + }) + } +} diff --git a/pkg/detector/helm/helm_detect.go b/pkg/detector/helm/helm_detect.go new file mode 100644 index 00000000000..0abe1f244d9 --- /dev/null +++ b/pkg/detector/helm/helm_detect.go @@ -0,0 +1,187 @@ +package helm + +import ( + "fmt" + "sort" + "strconv" + "strings" + + "github.com/Checkmarx/kics/pkg/detector" + "github.com/Checkmarx/kics/pkg/model" + "github.com/agnivade/levenshtein" + "github.com/rs/zerolog" +) + +// DetectKindLine defines a kindDetectLine type +type DetectKindLine struct { +} + +type detectCurlLine struct { + foundRes bool + lineRes int + breakRes bool + lastUnique dupHistory +} + +// dupHistory keeps the history of uniques +type dupHistory struct { + unique bool + lastUniqueLine int +} + +const ( + undetectedVulnerabilityLine = -1 +) + +// DetectLine is used to detect line on the helm template, +// it looks only at the keys of the template and will make use of the auxiliary added +// lines (ex: "# KICS_HELM_ID_") +func (d DetectKindLine) DetectLine(file *model.FileMetadata, searchKey string, + logWithFields *zerolog.Logger, outputLines int) model.VulnerabilityLines { + searchKey = fmt.Sprintf("%s.%s", strings.TrimRight(strings.TrimLeft(file.HelmID, "# "), ":"), searchKey) + text := strings.ReplaceAll(file.OriginalData, "\r", "") + lines := strings.Split(text, "\n") + curLineRes := detectCurlLine{ + foundRes: false, + lineRes: 0, + breakRes: false, + } + var extractedString [][]string + extractedString = detector.GetBracketValues(searchKey, extractedString, "") + sanitizedSubstring := searchKey + for idx, str := range extractedString { + sanitizedSubstring = strings.Replace(sanitizedSubstring, str[0], `{{`+strconv.Itoa(idx)+`}}`, -1) + } + + helmID, err := strconv.Atoi(strings.TrimSuffix(strings.TrimPrefix(file.HelmID, "# KICS_HELM_ID_"), ":")) + if err != nil { + helmID = -1 + } + + // Since we are only looking at keys we can ignore the second value passed through '=' and '[]' + for _, key := range strings.Split(sanitizedSubstring, ".") { + substr1, _ := detector.GenerateSubstrings(key, extractedString) + curLineRes = curLineRes.detectCurrentLine(lines, fmt.Sprintf("%s:", substr1), "", true, file.IDInfo, helmID) + + if curLineRes.breakRes { + break + } + } + + // Look at dupHistory to see if the last element was duplicate, if so + // change the line to the last unique key + if !curLineRes.lastUnique.unique { + curLineRes.lineRes = curLineRes.lastUnique.lastUniqueLine + } + + if curLineRes.foundRes { + lineRemove := make(map[int]int) + count := 0 + for i, line := range lines { // Remove auxiliary lines + if strings.Contains(line, "# KICS_HELM_ID_") { + count++ + lineRemove[i] = count + lines = append(lines[:i], lines[i+1:]...) + } + } + // Update found line + curLineRes.lineRes = removeLines(curLineRes.lineRes, lineRemove) + return model.VulnerabilityLines{ + Line: curLineRes.lineRes + 1, + VulnLines: detector.GetAdjacentVulnLines(curLineRes.lineRes, outputLines, lines), + LineWithVulnerabilty: strings.Split(lines[curLineRes.lineRes], ": ")[0], + } + } + + logWithFields.Warn().Msgf("Failed to detect line, query response %s", searchKey) + + return model.VulnerabilityLines{ + Line: undetectedVulnerabilityLine, + VulnLines: []model.CodeLine{}, + } +} + +// removeLines is used to update the vulnerability line after removing the "# KICS_HELM_ID_" +func removeLines(current int, lineRemove map[int]int) int { + orderByKey := make([]int, len(lineRemove)) + i := 0 + for k := range lineRemove { + orderByKey[i] = k + i++ + } + remove := 0 + sort.Ints(orderByKey) + for _, k := range orderByKey { + if current > k { + remove = lineRemove[k] + } else { + break + } + } + current -= remove + return current +} + +func (d detectCurlLine) detectCurrentLine(lines []string, str1, + str2 string, byKey bool, idInfo map[int]interface{}, id int) detectCurlLine { + distances := make(map[int]int) + for i := d.lineRes; i < len(lines); i++ { + if str1 != "" && str2 != "" { + if strings.Contains(lines[i], str1) && strings.Contains(lines[i], str2) { + distances[i] = levenshtein.ComputeDistance(detector.ExtractLineFragment(lines[i], str2, byKey), str2) + } + } else if str1 != "" { + if strings.Contains(lines[i], str1) { + distances[i] = levenshtein.ComputeDistance( + detector.ExtractLineFragment(strings.TrimSpace(lines[i]), str1, byKey), str1) + } + } + } + + lastSingle := d.lastUnique.lastUniqueLine + + if len(distances) == 0 { + return detectCurlLine{ + foundRes: d.foundRes, + lineRes: d.lineRes, + breakRes: true, + lastUnique: dupHistory{ + lastUniqueLine: lastSingle, + unique: d.lastUnique.unique, + }, + } + } + + lineResponse := detector.SelectLineWithMinimumDistance(distances, d.lineRes) + // if lineResponse is unique + unique := detectLastSingle(lineResponse, distances, idInfo, id) + if unique { + lastSingle = lineResponse + } + + return detectCurlLine{ + foundRes: true, + lineRes: lineResponse, + breakRes: false, + lastUnique: dupHistory{ + unique: unique, + lastUniqueLine: lastSingle, + }, + } +} + +// detectLastSingle checks if the line is unique or a duplicate +func detectLastSingle(line int, dis map[int]int, idInfo map[int]interface{}, id int) bool { + if idInfo == nil { + return true + } + for key, value := range dis { + if value == dis[line] && key != line { + // check if we are only looking at original data equivalent to the vulnerability + if ok := idInfo[id].(map[int]int)[key]; ok != 0 { + return false + } + } + } + return true +} diff --git a/pkg/detector/helm/helm_detect_test.go b/pkg/detector/helm/helm_detect_test.go new file mode 100644 index 00000000000..c33c29544f9 --- /dev/null +++ b/pkg/detector/helm/helm_detect_test.go @@ -0,0 +1,193 @@ +package helm + +import ( + "reflect" + "testing" + + "github.com/Checkmarx/kics/pkg/model" + "github.com/rs/zerolog" +) + +func TestEngine_detectHelmLine(t *testing.T) { //nolint + type args struct { + file *model.FileMetadata + searchKey string + logWithFields *zerolog.Logger + outputLines int + } + + tests := []struct { + name string + args args + want model.VulnerabilityLines + }{ + { + name: "test_detect_helm_line", + args: args{ + file: &model.FileMetadata{ + ID: "1", + ScanID: "console", + Document: model.Document{}, + Kind: model.KindHELM, + FileName: "test-connection.yaml", + HelmID: "# KICS_HELM_ID_0", + OriginalData: `# KICS_HELM_ID_0: +apiVersion: v1 +kind: Pod +metadata: + name: "{{ include "test_helm.fullname" . }}-test-connection" + labels: + {{- include "test_helm.labels" . | nindent 4 }} + annotations: + "helm.sh/hook": test +spec: + containers: + - name: wget + image: busybox + command: ['wget'] + args: ['{{ include "test_helm.fullname" . }}:{{ .Values.service.port }}'] + restartPolicy: Never +`, + Content: ``, + }, + searchKey: "KICS_HELM_ID_0.metadata.name={{RELEASE-NAME-test_helm-test-connection}}.spec.containers", + logWithFields: &zerolog.Logger{}, + outputLines: 1, + }, + want: model.VulnerabilityLines{ + Line: 10, + VulnLines: []model.CodeLine{ + { + Position: 10, + Line: " containers:", + }, + }, + LineWithVulnerabilty: " containers:", + }, + }, + { + name: "test_dup_values", + args: args{ + file: &model.FileMetadata{ + ID: "1", + ScanID: "console", + Document: model.Document{}, + Kind: model.KindHELM, + FileName: "test-dup_values.yaml", + IDInfo: map[int]interface{}{0: map[int]int{0: 0, 1: 1, 2: 2, 3: 3, 4: 4, + 5: 5, 6: 6, 7: 7, 8: 8, 9: 9, 10: 10, 11: 11, 12: 12, 13: 13, 14: 14, 15: 15, 16: 16, 17: 17, + 18: 18, 19: 19, 21: 21, 22: 22}}, + HelmID: "# KICS_HELM_ID_0", + OriginalData: `# KICS_HELM_ID_0: +apiVersion: v1 +kind: Pod +metadata: + name: "{{ include "test_helm.fullname" . }}-test-connection" + labels: + {{- include "test_helm.labels" . | nindent 4 }} + annotations: + "helm.sh/hook": test +spec: + containers: + - name: wget + image: busybox + command: ['wget'] + args: ['{{ include "test_helm.fullname" . }}:{{ .Values.service.port }}'] + restartPolicy: Never + containers: + - name: wget2 + image: busybox + command: ['wget'] + args: ['{{ include "test_helm.fullname" . }}:{{ .Values.service.port }}'] + restartPolicy: Never +`, + Content: ``, + }, + searchKey: "KICS_HELM_ID_0.metadata.name={{RELEASE-NAME-test_helm-test-connection}}.spec.containers", + logWithFields: &zerolog.Logger{}, + outputLines: 1, + }, + want: model.VulnerabilityLines{ + Line: 9, + VulnLines: []model.CodeLine{ + { + Position: 9, + Line: "spec:", + }, + }, + LineWithVulnerabilty: "spec:", + }, + }, + { + name: "test_detect_helm_with_dups", + args: args{ + file: &model.FileMetadata{ + ID: "1", + ScanID: "console", + Document: model.Document{}, + Kind: model.KindHELM, + FileName: "test-dups.yaml", + HelmID: "# KICS_HELM_ID_1", + OriginalData: `# KICS_HELM_ID_0: +apiVersion: v1 +kind: Pod +metadata: + name: "{{ include "test_helm.fullname" . }}-test-connection" + labels: + {{- include "test_helm.labels" . | nindent 4 }} + annotations: + "helm.sh/hook": test +spec: + containers: + - name: wget + image: busybox + command: ['wget'] + args: ['{{ include "test_helm.fullname" . }}:{{ .Values.service.port }}'] + restartPolicy: Never +--- +# KICS_HELM_ID_1: +apiVersion: v1 +kind: Pod +metadata: + name: "{{ include "test_helm.fullname" . }}-test-dups" + labels: + {{- include "test_helm.labels" . | nindent 4 }} + annotations: + "helm.sh/hook": test +spec: + containers: + - name: wget + image: busybox + command: ['wget'] + args: ['{{ include "test_helm.fullname" . }}:{{ .Values.service.port }}'] + restartPolicy: Never +`, + Content: ``, + }, + searchKey: "KICS_HELM_ID_1.metadata.name={{RELEASE-NAME-test_helm-test-connection}}.spec.containers", + logWithFields: &zerolog.Logger{}, + outputLines: 1, + }, + want: model.VulnerabilityLines{ + Line: 26, + VulnLines: []model.CodeLine{ + { + Position: 26, + Line: " containers:", + }, + }, + LineWithVulnerabilty: " containers:", + }, + }, + } + + for _, tt := range tests { + detector := DetectKindLine{} + t.Run(tt.name, func(t *testing.T) { + got := detector.DetectLine(tt.args.file, tt.args.searchKey, tt.args.logWithFields, tt.args.outputLines) + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("detectHelmLine() = %v, want = %v", got, tt.want) + } + }) + } +} diff --git a/pkg/detector/helper.go b/pkg/detector/helper.go new file mode 100644 index 00000000000..c3d1454aa56 --- /dev/null +++ b/pkg/detector/helper.go @@ -0,0 +1,222 @@ +package detector + +import ( + "fmt" + "regexp" + "strconv" + "strings" + + "github.com/Checkmarx/kics/internal/constants" + "github.com/Checkmarx/kics/pkg/model" + "github.com/agnivade/levenshtein" +) + +var ( + nameRegex = regexp.MustCompile(`^([A-Za-z0-9-_]+)\[([A-Za-z0-9-_{}]+)]$`) + nameRegexDocker = regexp.MustCompile(`{{(.*?)}}`) +) + +const ( + namePartsLength = 3 + valuePartsLength = 2 +) + +// GetBracketValues gets values inside "{{ }}" ignoring any "{{" or "}}" inside +func GetBracketValues(expr string, list [][]string, restOfString string) [][]string { + var tempList []string + firstOpen := strings.Index(expr, "{{") + firstClose := strings.Index(expr, "}}") + switchVal := firstClose - firstOpen + if switchVal == 0 { // if there is no "{{" and no "}}" + if expr != "" { + tempList = append(tempList, fmt.Sprintf("{{%s}}", expr), expr) + list = append(list, tempList) + } + if restOfString == "" { + return list // if there is no more string to read from return value of list + } + list = GetBracketValues(restOfString, list, "") // recursive call to the rest of the string + } else if switchVal > 0 { // if the position of the first "}}" is bigger than than the position of "{{" + // recursive with the value inside of curly brackets + list = GetBracketValues(expr[firstOpen+2:firstClose], list, expr[firstClose+2:]) + } else { // if the position of the first "{{" is bigger than than the position of "}}" + nextClose := strings.Index(restOfString, "}}") + tempList = append(tempList, fmt.Sprintf("{{%s%s}}", expr, restOfString[nextClose:]), + fmt.Sprintf("%s%s", expr, restOfString[nextClose:])) + list = append(list, tempList) + list = GetBracketValues(restOfString[nextClose+2:], list, "") // recursive call to the rest of the string + } + return list +} + +// GenerateSubstrings returns the substrings used for line searching depending on search key +// '.' is new line +// '=' is value in the same line +// '[]' is in the same line +func GenerateSubstrings(key string, extractedString [][]string) (substr1Res, substr2Res string) { + var substr1, substr2 string + if parts := nameRegex.FindStringSubmatch(key); len(parts) == namePartsLength { + substr1, substr2 = getKeyWithCurlyBrackets(key, extractedString, parts) + } else if parts := strings.Split(key, "="); len(parts) == valuePartsLength { + substr1, substr2 = getKeyWithCurlyBrackets(key, extractedString, parts) + } else { + parts := []string{key, ""} + substr1, substr2 = getKeyWithCurlyBrackets(key, extractedString, parts) + } + return substr1, substr2 +} + +func getKeyWithCurlyBrackets(key string, extractedString [][]string, parts []string) (substr1Res, substr2Res string) { + var substr1, substr2 string + extractedPart := nameRegexDocker.FindStringSubmatch(key) + if len(extractedPart) == valuePartsLength { + for idx, key := range parts { + if extractedPart[0] == key { + switch idx { + case (len(parts) - 2): + i, _ := strconv.Atoi(extractedPart[1]) + substr1 = extractedString[i][1] + case len(parts) - 1: + i, _ := strconv.Atoi(extractedPart[1]) + substr2 = extractedString[i][1] + } + } else { + substr1 = generateSubstr(substr1, parts, 2) + substr2 = generateSubstr(substr2, parts, 1) + } + } + } else { + substr1 = parts[len(parts)-2] + substr2 = parts[len(parts)-1] + } + + return substr1, substr2 +} + +func generateSubstr(substr string, parts []string, leng int) string { + if substr == "" { + substr = parts[len(parts)-leng] + } + return substr +} + +// GetAdjacentVulnLines is used to get the lines adjecent to the line that contains the vulnerability +// adj is the amount of lines wanted +func GetAdjacentVulnLines(idx, adj int, lines []string) []model.CodeLine { + var endPos int + var startPos int + if adj <= len(lines) { + endPos = idx + adj/2 + 1 // if adj lines passes the number of lines in file + if len(lines) < endPos { + endPos = len(lines) + } + startAdj := adj + if adj%2 == 0 { + startAdj-- + } + + startPos = idx - startAdj/2 // if adj lines passes the first line in the file + if startPos < 0 { + startPos = 0 + } + } else { // in case adj is bigger than number of lines in file + adj = len(lines) + endPos = len(lines) + startPos = 0 + } + + switch idx { + case 0: + // case vulnerability is the first line of the file + return createVulnLines(1, lines[:adj]) + case len(lines) - 1: + // case vulnerability is the last line of the file + return createVulnLines(startPos+1, lines[len(lines)-adj:]) + default: + // case vulnerability is in the midle of the file + return createVulnLines(startPos+1, lines[startPos:endPos]) + } +} + +// createVulnLines is the function that will generate the array that contains the lines numbers +// used to alter the color of the line that contains the vulnerability +func createVulnLines(startPos int, lines []string) []model.CodeLine { + vulns := make([]model.CodeLine, len(lines)) + for idx, line := range lines { + vulns[idx] = model.CodeLine{ + Line: line, + Position: startPos, + } + startPos++ + } + return vulns +} + +// SelectLineWithMinimumDistance will search a map of levenshtein distances to find the minimum distance +func SelectLineWithMinimumDistance(distances map[int]int, startingFrom int) int { + minDistance, lineOfMinDistance := constants.MaxInteger, startingFrom + for line, distance := range distances { + if distance < minDistance || distance == minDistance && line < lineOfMinDistance { + minDistance = distance + lineOfMinDistance = line + } + } + + return lineOfMinDistance +} + +// ExtractLineFragment will prepare substr for line detection +func ExtractLineFragment(line, substr string, key bool) string { + // If detecting line by keys only + if key { + return line[:strings.Index(line, ":")] + } + start := strings.Index(line, substr) + end := start + len(substr) + + for start >= 0 { + if line[start] == ' ' { + break + } + + start-- + } + + for end < len(line) { + if line[end] == ' ' { + break + } + + end++ + } + + result := line[start+1 : end] + // workaround for selecting yaml keys + if result[len(result)-1] == ':' { + end-- + } + return line[start+1 : end] +} + +// DetectCurrentLine uses levenshtein distance to find the most acurate line for the vulnerability +func DetectCurrentLine(lines []string, str1, str2 string, + curLine int, foundOne bool) (foundRes bool, lineRes int, breakRes bool) { + distances := make(map[int]int) + for i := curLine; i < len(lines); i++ { + if str1 != "" && str2 != "" { + if strings.Contains(lines[i], str1) && strings.Contains(lines[i], str2) { + distances[i] = levenshtein.ComputeDistance(ExtractLineFragment(lines[i], str2, false), str2) + } + } else if str1 != "" { + if strings.Contains(lines[i], str1) { + distances[i] = levenshtein.ComputeDistance(ExtractLineFragment(lines[i], str1, false), str1) + } + } + } + + if len(distances) == 0 { + return foundOne, curLine, true + } + + return true, SelectLineWithMinimumDistance(distances, curLine), false +} diff --git a/pkg/detector/helper_test.go b/pkg/detector/helper_test.go new file mode 100644 index 00000000000..9ded0b25b18 --- /dev/null +++ b/pkg/detector/helper_test.go @@ -0,0 +1,332 @@ +package detector + +import ( + "fmt" + "reflect" + "testing" + + "github.com/Checkmarx/kics/pkg/model" + "github.com/Checkmarx/kics/test" + "github.com/stretchr/testify/require" +) + +// TestSelectLineWithMinimumDistance tests the functions [SelectLineWithMinimumDistance()] and all the methods called by them +func TestSelectLineWithMinimumDistance(t *testing.T) { + values := []struct { + distances map[int]int + startingFrom int + expectedResult int + }{ + { + distances: map[int]int{ + 12: 0, + }, + startingFrom: 0, + expectedResult: 12, + }, + { + distances: map[int]int{ + 12: 0, + 24: 0, + }, + startingFrom: 11, + expectedResult: 12, + }, + { + distances: map[int]int{ + 1: 26, + 2: 5, + 3: 0, + }, + startingFrom: 1, + expectedResult: 3, + }, + } + + for i, testCase := range values { + t.Run(fmt.Sprintf("selectLineWithMinimumDistance-%d", i), func(t *testing.T) { + v := SelectLineWithMinimumDistance(testCase.distances, testCase.startingFrom) + require.Equal(t, testCase.expectedResult, v) + }) + } +} + +// TestGetBracketValues tests the functions [getBracketValues()] and all the methods called by them +func TestGetBracketValues(t *testing.T) { + type args struct { + expr string + } + tests := []struct { + name string + args args + want [][]string + }{ + { + name: "no_brackets", + args: args{ + expr: "password", + }, + want: [][]string{ + { + "{{password}}", + "password", + }, + }, + }, + { + name: "single_brackets", + args: args{ + expr: "{{password}}", + }, + want: [][]string{ + { + "{{password}}", + "password", + }, + }, + }, + { + name: "double_brackets", + args: args{ + expr: "{{ {{password}} }}", + }, + want: [][]string{ + { + "{{ {{password}}}}", + " {{password}}", + }, + }, + }, + { + name: "multiple_brackets", + args: args{ + expr: "FROM={{open-jdk}}.{{ {{password}} }}", + }, + want: [][]string{ + { + "{{open-jdk}}", + "open-jdk", + }, + { + "{{ {{password}}}}", + " {{password}}", + }, + }, + }, + } + + for _, tt := range tests { + var got [][]string + t.Run(tt.name, func(t *testing.T) { + got = GetBracketValues(tt.args.expr, got, "") + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("DefaultVulnerabilityBuilder() = %v, want %v", got, tt.want) + } + }) + } +} + +// TestGetAdjacents tests the functions [GetAdjacents()] and all the methods called by them +func TestGetAdjacents(t *testing.T) { //nolint + type args struct { + idx int + adj int + lines []string + } + tests := []struct { + name string + args args + want []model.CodeLine + }{ + { + name: "test_start_of_file", + args: args{ + idx: 0, + adj: 3, + lines: []string{ + "firstline", + "secondline", + "thirdline", + "forthline", + }, + }, + want: []model.CodeLine{ + { + Position: 1, + Line: "firstline", + }, + { + Position: 2, + Line: "secondline", + }, + { + Position: 3, + Line: "thirdline", + }, + }, + }, + { + name: "test_end_of_file", + args: args{ + idx: 3, + adj: 3, + lines: []string{ + "firstline", + "secondline", + "thirdline", + "forthline", + }, + }, + want: []model.CodeLine{ + { + Position: 3, + Line: "secondline", + }, + { + Position: 4, + Line: "thirdline", + }, + { + Position: 5, + Line: "forthline", + }, + }, + }, + { + name: "test_midle_of_file", + args: args{ + idx: 1, + adj: 3, + lines: []string{ + "firstline", + "secondline", + "thirdline", + "forthline", + }, + }, + want: []model.CodeLine{ + { + Position: 1, + Line: "firstline", + }, + { + Position: 2, + Line: "secondline", + }, + { + Position: 3, + Line: "thirdline", + }, + }, + }, + { + name: "test_even_adj", + args: args{ + idx: 1, + adj: 2, + lines: []string{ + "firstline", + "secondline", + "thirdline", + "forthline", + }, + }, + want: []model.CodeLine{ + { + Position: 2, + Line: "secondline", + }, + { + Position: 3, + Line: "thirdline", + }, + }, + }, + { + name: "test_even_adj_first_line", + args: args{ + idx: 0, + adj: 2, + lines: []string{ + "firstline", + "secondline", + "thirdline", + "forthline", + }, + }, + want: []model.CodeLine{ + { + Position: 1, + Line: "firstline", + }, + { + Position: 2, + Line: "secondline", + }, + }, + }, + { + name: "test_one_adj", + args: args{ + idx: 3, + adj: 1, + lines: []string{ + "firstline", + "secondline", + "thirdline", + "forthline", + }, + }, + want: []model.CodeLine{ + { + Position: 4, + Line: "forthline", + }, + }, + }, + { + name: "test_adj_bigger_than_file", + args: args{ + idx: 3, + adj: 5, + lines: []string{ + "firstline", + "secondline", + "thirdline", + "forthline", + }, + }, + want: []model.CodeLine{ + { + Position: 1, + Line: "firstline", + }, + { + Position: 2, + Line: "secondline", + }, + { + Position: 3, + Line: "thirdline", + }, + { + Position: 4, + Line: "forthline", + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := GetAdjacentVulnLines(tt.args.idx, tt.args.adj, tt.args.lines) + gotStrVulnerabilities, err := test.StringifyStruct(got) + require.Nil(t, err) + wantStrVulnerabilities, err := test.StringifyStruct(tt.want) + require.Nil(t, err) + if !reflect.DeepEqual(gotStrVulnerabilities, wantStrVulnerabilities) { + t.Errorf("getAdjacents() = %v, want = %v", gotStrVulnerabilities, wantStrVulnerabilities) + } + }) + } +} diff --git a/pkg/engine/inspector.go b/pkg/engine/inspector.go index 5c3ef6a4950..10653ae9f15 100644 --- a/pkg/engine/inspector.go +++ b/pkg/engine/inspector.go @@ -9,6 +9,9 @@ import ( "time" consoleHelpers "github.com/Checkmarx/kics/internal/console/helpers" + "github.com/Checkmarx/kics/pkg/detector" + "github.com/Checkmarx/kics/pkg/detector/docker" + "github.com/Checkmarx/kics/pkg/detector/helm" "github.com/Checkmarx/kics/pkg/engine/source" "github.com/Checkmarx/kics/pkg/model" "github.com/getsentry/sentry-go" @@ -40,7 +43,8 @@ var ErrNoResult = errors.New("query: not result") var ErrInvalidResult = errors.New("query: invalid result format") // VulnerabilityBuilder represents a function that will build a vulnerability -type VulnerabilityBuilder func(ctx *QueryContext, tracker Tracker, v interface{}) (model.Vulnerability, error) +type VulnerabilityBuilder func(ctx *QueryContext, tracker Tracker, v interface{}, + detector *detector.DetectLine) (model.Vulnerability, error) // Tracker wraps an interface that contain basic methods: TrackQueryLoad, TrackQueryExecution and FailedDetectLine // TrackQueryLoad increments the number of loaded queries @@ -68,6 +72,7 @@ type Inspector struct { tracker Tracker failedQueries map[string]error excludeResults map[string]bool + detector *detector.DetectLine enableCoverageReport bool coverageReport cover.Report @@ -157,12 +162,17 @@ func NewInspector( log.Info(). Msgf("Inspector initialized, number of queries=%d", queriesNumber) + lineDetctor := detector.NewDetectLine(tracker.GetOutputLines()). + Add(helm.DetectKindLine{}, model.KindHELM). + Add(docker.DetectKindLine{}, model.KindDOCKER) + return &Inspector{ queries: opaQueries, vb: vb, tracker: tracker, failedQueries: failedQueries, excludeResults: excludeResults, + detector: lineDetctor, }, nil } @@ -310,7 +320,7 @@ func (c *Inspector) decodeQueryResults(ctx *QueryContext, results rego.ResultSet vulnerabilities := make([]model.Vulnerability, 0, len(queryResultItems)) failedDetectLine := false for _, queryResultItem := range queryResultItems { - vulnerability, err := c.vb(ctx, c.tracker, queryResultItem) + vulnerability, err := c.vb(ctx, c.tracker, queryResultItem, c.detector) if err != nil { sentry.CaptureException(err) log.Err(err). diff --git a/pkg/engine/inspector_test.go b/pkg/engine/inspector_test.go index 24df144dfcf..f820feb0ad3 100644 --- a/pkg/engine/inspector_test.go +++ b/pkg/engine/inspector_test.go @@ -11,6 +11,9 @@ import ( "github.com/Checkmarx/kics/internal/tracker" + "github.com/Checkmarx/kics/pkg/detector" + "github.com/Checkmarx/kics/pkg/detector/docker" + "github.com/Checkmarx/kics/pkg/detector/helm" "github.com/Checkmarx/kics/pkg/engine/source" "github.com/Checkmarx/kics/pkg/model" "github.com/Checkmarx/kics/test" @@ -110,6 +113,9 @@ func TestInspector_GetCoverageReport(t *testing.T) { // TestInspect tests the functions [Inspect()] and all the methods called by them func TestInspect(t *testing.T) { //nolint + inspDetector := detector.NewDetectLine(3). + Add(helm.DetectKindLine{}, model.KindHELM). + Add(docker.DetectKindLine{}, model.KindDOCKER) ctx := context.Background() opaQuery, _ := rego.New( rego.Query(regoQuery), @@ -240,6 +246,7 @@ func TestInspect(t *testing.T) { //nolint QueryURI: "https://github.com/Checkmarx/kics/", Severity: model.SeverityInfo, Line: -1, + VulnLines: []model.CodeLine{}, IssueType: "IncorrectValue", SearchKey: "{{ADD ${JAR_FILE} app.jar}}", KeyExpectedValue: "'COPY' app.jar", @@ -288,6 +295,7 @@ func TestInspect(t *testing.T) { //nolint enableCoverageReport: tt.fields.enableCoverageReport, coverageReport: tt.fields.coverageReport, excludeResults: tt.fields.excludeResults, + detector: inspDetector, } got, err := c.Inspect(tt.args.ctx, tt.args.scanID, tt.args.files, true, filepath.FromSlash("assets/queries/")) if tt.wantErr { diff --git a/pkg/engine/vulnerability_builder.go b/pkg/engine/vulnerability_builder.go index 76cd0f26360..317cf6d6bb5 100644 --- a/pkg/engine/vulnerability_builder.go +++ b/pkg/engine/vulnerability_builder.go @@ -3,48 +3,16 @@ package engine import ( "encoding/json" "fmt" - "regexp" - "sort" "strconv" "strings" + "github.com/Checkmarx/kics/pkg/detector" "github.com/Checkmarx/kics/pkg/model" - "github.com/agnivade/levenshtein" "github.com/pkg/errors" "github.com/rs/zerolog" "github.com/rs/zerolog/log" ) -var ( - nameRegex = regexp.MustCompile(`^([A-Za-z0-9-_]+)\[([A-Za-z0-9-_{}]+)]$`) - nameRegexDocker = regexp.MustCompile(`{{(.*?)}}`) - nameRegexDockerFileML = regexp.MustCompile(`.+\s+\\$`) -) - -const ( - namePartsLength = 3 - valuePartsLength = 2 -) - -type vulnerabilityLines struct { - line int - vulnLine model.VulnLines - lineWithVulnerabilty string -} - -type detectCurlLine struct { - foundRes bool - lineRes int - breakRes bool - lastUnique dupHistory -} - -// dupHistory keeps the history of uniques -type dupHistory struct { - unique bool - lastUniqueLine int -} - func getStringFromMap(vulnParam, defaultParam string, vOjb map[string]interface{}, logWithFields *zerolog.Logger) string { ts, err := mapKeyToString(vOjb, vulnParam, false) if err != nil { @@ -56,7 +24,8 @@ func getStringFromMap(vulnParam, defaultParam string, vOjb map[string]interface{ } // DefaultVulnerabilityBuilder defines a vulnerability builder to execute default actions of scan -var DefaultVulnerabilityBuilder = func(ctx *QueryContext, tracker Tracker, v interface{}) (model.Vulnerability, error) { +var DefaultVulnerabilityBuilder = func(ctx *QueryContext, tracker Tracker, + v interface{}, detector *detector.DetectLine) (model.Vulnerability, error) { vObj, ok := v.(map[string]interface{}) if !ok { return model.Vulnerability{}, ErrInvalidResult @@ -90,23 +59,17 @@ var DefaultVulnerabilityBuilder = func(ctx *QueryContext, tracker Tracker, v int Str("queryName", ctx.query.metadata.Query). Logger() - linesVulne := vulnerabilityLines{ - line: UndetectedVulnerabilityLine, - vulnLine: model.VulnLines{}, + detector.SetupLogs(&logWithFields) + + linesVulne := model.VulnerabilityLines{ + Line: -1, + VulnLines: []model.CodeLine{}, } + searchKey := "" if s, ok := vObj["searchKey"]; ok { searchKey = s.(string) - switch file.Kind { - case model.KindDOCKER: - linesVulne = detectDockerLine(&file, searchKey, &logWithFields, tracker.GetOutputLines()) - case model.KindHELM: - // Update search key to make use of the auxiliary lines - tempSearchKey := fmt.Sprintf("%s.%s", strings.TrimRight(strings.TrimLeft(file.HelmID, "# "), ":"), searchKey) - linesVulne = detectHelmLine(&file, tempSearchKey, &logWithFields, tracker.GetOutputLines()) - default: - linesVulne = detectLine(&file, searchKey, &logWithFields, tracker.GetOutputLines()) - } + linesVulne = detector.DetectLine(&file, searchKey) } else { logWithFields.Error().Msg("Saving result. failed to detect line") } @@ -165,8 +128,8 @@ var DefaultVulnerabilityBuilder = func(ctx *QueryContext, tracker Tracker, v int Description: getStringFromMap("descriptionText", "", vObj, &logWithFields), Severity: severity, Platform: getStringFromMap("platform", "", vObj, &logWithFields), - Line: linesVulne.line, - VulnLines: linesVulne.vulnLine, + Line: linesVulne.Line, + VulnLines: linesVulne.VulnLines, IssueType: issueType, SearchKey: searchKey, SearchValue: searchValue, @@ -189,288 +152,6 @@ func mergeWithMetadata(base, additional map[string]interface{}) map[string]inter return base } -/* - detectHelmLine is used to detect line on the helm template, - it looks only at the keys of the template and will make use of the auxiliary added - lines (ex: "# KICS_HELM_ID_") -*/ -func detectHelmLine(file *model.FileMetadata, searchKey string, logWithFields *zerolog.Logger, outputLines int) vulnerabilityLines { - text := strings.ReplaceAll(file.OriginalData, "\r", "") - lines := strings.Split(text, "\n") - curLineRes := detectCurlLine{ - foundRes: false, - lineRes: 0, - breakRes: false, - } - var extractedString [][]string - extractedString = getBracketValues(searchKey, extractedString, "") - sanitizedSubstring := searchKey - for idx, str := range extractedString { - sanitizedSubstring = strings.Replace(sanitizedSubstring, str[0], `{{`+strconv.Itoa(idx)+`}}`, -1) - } - - helmID, err := strconv.Atoi(strings.TrimSuffix(strings.TrimPrefix(file.HelmID, "# KICS_HELM_ID_"), ":")) - if err != nil { - helmID = -1 - } - - // Since we are only looking at keys we can ignore the second value passed through '=' and '[]' - for _, key := range strings.Split(sanitizedSubstring, ".") { - substr1, _ := generateSubstrings(key, extractedString) - curLineRes = curLineRes.detectCurrentLine(lines, fmt.Sprintf("%s:", substr1), "", true, file.IDInfo, helmID) - - if curLineRes.breakRes { - break - } - } - - // Look at dupHistory to see if the last element was duplicate, if so - // change the line to the last unique key - if !curLineRes.lastUnique.unique { - curLineRes.lineRes = curLineRes.lastUnique.lastUniqueLine - } - - if curLineRes.foundRes { - lineRemove := make(map[int]int) - count := 0 - for i, line := range lines { // Remove auxiliary lines - if strings.Contains(line, "# KICS_HELM_ID_") { - count++ - lineRemove[i] = count - lines = append(lines[:i], lines[i+1:]...) - } - } - // Update found line - curLineRes.lineRes = removeLines(curLineRes.lineRes, lineRemove) - return vulnerabilityLines{ - line: curLineRes.lineRes + 1, - vulnLine: getAdjacentLines(curLineRes.lineRes, outputLines, lines), - lineWithVulnerabilty: strings.Split(lines[curLineRes.lineRes], ": ")[0], - } - } - - logWithFields.Warn().Msgf("failed to detect line, query response %s", searchKey) - - return vulnerabilityLines{ - line: UndetectedVulnerabilityLine, - vulnLine: model.VulnLines{}, - } -} - -// removeLines is used to update the vulnerability line after removing the "# KICS_HELM_ID_" -func removeLines(current int, lineRemove map[int]int) int { - orderByKey := make([]int, len(lineRemove)) - i := 0 - for k := range lineRemove { - orderByKey[i] = k - i++ - } - remove := 0 - sort.Ints(orderByKey) - for _, k := range orderByKey { - if current > k { - remove = lineRemove[k] - } else { - break - } - } - current -= remove - return current -} - -func detectDockerLine(file *model.FileMetadata, searchKey string, logWithFields *zerolog.Logger, outputLines int) vulnerabilityLines { - text := strings.ReplaceAll(file.OriginalData, "\r", "") - lines := prepareDockerFileLines(text) - curLineRes := detectCurlLine{ - foundRes: false, - lineRes: 0, - breakRes: false, - } - var extractedString [][]string - extractedString = getBracketValues(searchKey, extractedString, "") - sKey := searchKey - for idx, str := range extractedString { - sKey = strings.Replace(sKey, str[0], `{{`+strconv.Itoa(idx)+`}}`, -1) - } - - for _, key := range strings.Split(sKey, ".") { - substr1, substr2 := generateSubstrings(key, extractedString) - - curLineRes = curLineRes.detectCurrentLine(lines, substr1, substr2, false, nil, -1) - - if curLineRes.breakRes { - break - } - } - - if curLineRes.foundRes { - return vulnerabilityLines{ - line: curLineRes.lineRes + 1, - vulnLine: getAdjacentLines(curLineRes.lineRes, outputLines, strings.Split(text, "\n")), - } - } - - logWithFields.Warn().Msgf("Failed to detect Docker line, query response %s", searchKey) - - return vulnerabilityLines{ - line: UndetectedVulnerabilityLine, - vulnLine: model.VulnLines{}, - } -} - -func detectLine(file *model.FileMetadata, searchKey string, logWithFields *zerolog.Logger, outputLines int) vulnerabilityLines { - text := strings.ReplaceAll(file.OriginalData, "\r", "") - lines := strings.Split(text, "\n") - curLineRes := detectCurlLine{ - foundRes: false, - lineRes: 0, - breakRes: false, - } - var extractedString [][]string - extractedString = getBracketValues(searchKey, extractedString, "") - sanitizedSubstring := searchKey - for idx, str := range extractedString { - sanitizedSubstring = strings.Replace(sanitizedSubstring, str[0], `{{`+strconv.Itoa(idx)+`}}`, -1) - } - - for _, key := range strings.Split(sanitizedSubstring, ".") { - substr1, substr2 := generateSubstrings(key, extractedString) - - curLineRes = curLineRes.detectCurrentLine(lines, substr1, substr2, false, nil, -1) - - if curLineRes.breakRes { - break - } - } - - if curLineRes.foundRes { - return vulnerabilityLines{ - line: curLineRes.lineRes + 1, - vulnLine: getAdjacentLines(curLineRes.lineRes, outputLines, lines), - lineWithVulnerabilty: lines[curLineRes.lineRes], - } - } - - logWithFields.Warn().Msgf("Failed to detect line, query response %s", searchKey) - - return vulnerabilityLines{ - line: UndetectedVulnerabilityLine, - vulnLine: model.VulnLines{}, - } -} - -// getAdjacent is used to get the lines adjecent to the line that contains the vulnerability -// adj is the amount of lines wanted -func getAdjacentLines(idx, adj int, lines []string) model.VulnLines { - var endPos int - var startPos int - if adj <= len(lines) { - endPos = idx + adj/2 + 1 // if adj lines passes the number of lines in file - if len(lines) < endPos { - endPos = len(lines) - } - startAdj := adj - if adj%2 == 0 { - startAdj-- - } - - startPos = idx - startAdj/2 // if adj lines passes the first line in the file - if startPos < 0 { - startPos = 0 - } - } else { // in case adj is bigger than number of lines in file - adj = len(lines) - endPos = len(lines) - startPos = 0 - } - - switch idx { - case 0: - return model.VulnLines{ // case vulnerability is the first line of the file - Positions: generatePosArr(adj, 1), - Lines: lines[:adj], - } - case len(lines) - 1: // case vulnerability is the last line of the file - return model.VulnLines{ - Positions: generatePosArr(adj, startPos+1), - Lines: lines[len(lines)-adj:], - } - default: - return model.VulnLines{ // case vulnerability is in the midle of the file - Positions: generatePosArr(adj, startPos+1), - Lines: lines[startPos:endPos], - } - } -} - -// generatePosArr is the function that will generate the array that contains the lines numbers -// used to alter the color of the line that contains the vulnerability -func generatePosArr(adj, start int) []int { - posArr := make([]int, adj) - for i := 0; i < adj; i++ { - posArr[i] = start - start++ - } - return posArr -} - -func generateSubstrings(key string, extractedString [][]string) (substr1Res, substr2Res string) { - var substr1, substr2 string - if parts := nameRegex.FindStringSubmatch(key); len(parts) == namePartsLength { - substr1, substr2 = getKeyWithCurlyBrackets(key, extractedString, parts) - } else if parts := strings.Split(key, "="); len(parts) == valuePartsLength { - substr1, substr2 = getKeyWithCurlyBrackets(key, extractedString, parts) - } else { - parts := []string{key, ""} - substr1, substr2 = getKeyWithCurlyBrackets(key, extractedString, parts) - } - return substr1, substr2 -} - -func selectLineWithMinimumDistance(distances map[int]int, startingFrom int) int { - minDistance, lineOfMinDistance := 1000000000000, startingFrom - for line, distance := range distances { - if distance < minDistance || distance == minDistance && line < lineOfMinDistance { - minDistance = distance - lineOfMinDistance = line - } - } - - return lineOfMinDistance -} - -func extractLineFragment(line, substr string, key bool) string { - // If detecting line by keys only - if key { - return line[:strings.Index(line, ":")] - } - start := strings.Index(line, substr) - end := start + len(substr) - - for start >= 0 { - if line[start] == ' ' { - break - } - - start-- - } - - for end < len(line) { - if line[end] == ' ' { - break - } - - end++ - } - - result := line[start+1 : end] - // workaround for selecting yaml keys - if result[len(result)-1] == ':' { - end-- - } - return line[start+1 : end] -} - func mustMapKeyToString(m map[string]interface{}, key string) *string { res, err := mapKeyToString(m, key, true) if err != nil && key != "value" { @@ -528,147 +209,3 @@ func ptrStringToString(v *string) string { } return *v } - -func getKeyWithCurlyBrackets(key string, extractedString [][]string, parts []string) (substr1Res, substr2Res string) { - var substr1, substr2 string - extractedPart := nameRegexDocker.FindStringSubmatch(key) - if len(extractedPart) == valuePartsLength { - for idx, key := range parts { - if extractedPart[0] == key { - switch idx { - case (len(parts) - 2): - i, _ := strconv.Atoi(extractedPart[1]) - substr1 = extractedString[i][1] - case len(parts) - 1: - i, _ := strconv.Atoi(extractedPart[1]) - substr2 = extractedString[i][1] - } - } else { - substr1 = generateSubstr(substr1, parts, 2) - substr2 = generateSubstr(substr2, parts, 1) - } - } - } else { - substr1 = parts[len(parts)-2] - substr2 = parts[len(parts)-1] - } - - return substr1, substr2 -} - -func generateSubstr(substr string, parts []string, leng int) string { - if substr == "" { - substr = parts[len(parts)-leng] - } - return substr -} - -func prepareDockerFileLines(text string) []string { - textSplit := strings.Split(text, "\n") - for idx, key := range textSplit { - textSplit[idx] = multiLineSpliter(textSplit, key, idx) - } - return textSplit -} - -func (d detectCurlLine) detectCurrentLine(lines []string, str1, - str2 string, byKey bool, idInfo map[int]interface{}, id int) detectCurlLine { - distances := make(map[int]int) - for i := d.lineRes; i < len(lines); i++ { - if str1 != "" && str2 != "" { - if strings.Contains(lines[i], str1) && strings.Contains(lines[i], str2) { - distances[i] = levenshtein.ComputeDistance(extractLineFragment(lines[i], str2, byKey), str2) - } - } else if str1 != "" { - if strings.Contains(lines[i], str1) { - distances[i] = levenshtein.ComputeDistance(extractLineFragment(strings.TrimSpace(lines[i]), str1, byKey), str1) - } - } - } - - lastSingle := d.lastUnique.lastUniqueLine - - if len(distances) == 0 { - return detectCurlLine{ - foundRes: d.foundRes, - lineRes: d.lineRes, - breakRes: true, - lastUnique: dupHistory{ - lastUniqueLine: lastSingle, - unique: d.lastUnique.unique, - }, - } - } - - lineResponse := selectLineWithMinimumDistance(distances, d.lineRes) - // if lineResponse is unique - unique := detectLastSingle(lineResponse, distances, idInfo, id) - if unique { - lastSingle = lineResponse - } - - return detectCurlLine{ - foundRes: true, - lineRes: lineResponse, - breakRes: false, - lastUnique: dupHistory{ - unique: unique, - lastUniqueLine: lastSingle, - }, - } -} - -// detectLastSingle checks if the line is unique or a duplicate -func detectLastSingle(line int, dis map[int]int, idInfo map[int]interface{}, id int) bool { - if idInfo == nil { - return true - } - for key, value := range dis { - if value == dis[line] && key != line { - // check if we are only looking at original data equivalent to the vulnerability - if ok := idInfo[id].(map[int]int)[key]; ok != 0 { - return false - } - } - } - return true -} - -func multiLineSpliter(textSplit []string, key string, idx int) string { - if nameRegexDockerFileML.MatchString(key) { - i := idx + 1 - for textSplit[i] == "" { - i++ - } - textSplit[idx] = strings.ReplaceAll(textSplit[idx], " \\", " "+textSplit[i]) - textSplit[i] = "" - textSplit[idx] = multiLineSpliter(textSplit, textSplit[idx], idx) - } - return textSplit[idx] -} - -// getBracketValues gets values inside "{{ }}" ignoring any "{{" or "}}" inside -func getBracketValues(expr string, list [][]string, restOfString string) [][]string { - var tempList []string - firstOpen := strings.Index(expr, "{{") - firstClose := strings.Index(expr, "}}") - switchVal := firstClose - firstOpen - if switchVal == 0 { // if there is no "{{" and no "}}" - if expr != "" { - tempList = append(tempList, fmt.Sprintf("{{%s}}", expr), expr) - list = append(list, tempList) - } - if restOfString == "" { - return list // if there is no more string to read from return value of list - } - list = getBracketValues(restOfString, list, "") // recursive call to the rest of the string - } else if switchVal > 0 { // if the position of the first "}}" is bigger than than the position of "{{" - list = getBracketValues(expr[firstOpen+2:firstClose], list, expr[firstClose+2:]) // recursive with the value inside of curly brackets - } else { // if the position of the first "{{" is bigger than than the position of "}}" - nextClose := strings.Index(restOfString, "}}") - tempList = append(tempList, fmt.Sprintf("{{%s%s}}", expr, restOfString[nextClose:]), fmt.Sprintf("%s%s", expr, restOfString[nextClose:])) - list = append(list, tempList) - list = getBracketValues(restOfString[nextClose+2:], list, "") // recursive call to the rest of the string - } - return list -} diff --git a/pkg/engine/vulnerability_builder_test.go b/pkg/engine/vulnerability_builder_test.go index f27d4947466..95f3b31b6a7 100644 --- a/pkg/engine/vulnerability_builder_test.go +++ b/pkg/engine/vulnerability_builder_test.go @@ -6,174 +6,11 @@ import ( "testing" "github.com/Checkmarx/kics/internal/tracker" + "github.com/Checkmarx/kics/pkg/detector" "github.com/Checkmarx/kics/pkg/model" - "github.com/Checkmarx/kics/test" - "github.com/rs/zerolog" "github.com/stretchr/testify/require" ) -// TestDetectDockerLine tests the functions [DetectDockerLine()] and all the methods called by them -func TestDetectDockerLine(t *testing.T) { //nolint - testCases := []struct { - expected vulnerabilityLines - searchKey string - ctx *QueryContext - file *model.FileMetadata - }{ - { - expected: vulnerabilityLines{ - line: 10, - vulnLine: model.VulnLines{ - Positions: []int{9, 10, 11}, - Lines: []string{ - "RUN apk update", - "RUN apk update && apk upgrade && apk add kubectl=1.20.0-r0 \\", - "\t&& rm -rf /var/cache/apk/*", - }, - }, - }, - searchKey: "FROM={{alpine:3.9}}.RUN={{apk update && apk upgrade && apk add kubectl=1.20.0-r0 \u0026\u0026 rm -rf /var/cache/apk/*}}", - ctx: &QueryContext{ - scanID: "Test2", - }, - file: &model.FileMetadata{ - ScanID: "Test2", - ID: "Test2", - Kind: model.KindDOCKER, - OriginalData: `FROM alpine:3.7 -RUN apk update \ - && apk upgrade \ - && apk add kubectl=1.20.0-r0 \ - && rm -rf /var/cache/apk/* -ENTRYPOINT ["kubectl"] - -FROM alpine:3.9 -RUN apk update -RUN apk update && apk upgrade && apk add kubectl=1.20.0-r0 \ - && rm -rf /var/cache/apk/* -ENTRYPOINT ["kubectl"] -`, - }, - }, - { - expected: vulnerabilityLines{ - line: 17, - vulnLine: model.VulnLines{ - Positions: []int{16, 17, 18}, - Lines: []string{ - "ARG JAR_FILE", - "ADD ${JAR_FILE} apps.jar", - "", - }, - }, - }, - searchKey: "FROM=openjdk:11-jdk.{{ADD ${JAR_FILE} apps.jar}}", - ctx: &QueryContext{ - scanID: "Test3", - }, - file: &model.FileMetadata{ - ScanID: "Test3", - ID: "Test3", - Kind: model.KindDOCKER, - OriginalData: `FROM openjdk:10-jdk -VOLUME /tmp -ADD http://source.file/package.file.tar.gz /temp -RUN tar -xjf /temp/package.file.tar.gz \ - && make -C /tmp/package.file \ - && rm /tmp/ package.file.tar.gz -ARG JAR_FILE -ADD ${JAR_FILE} app.jar - -FROM openjdk:11-jdk -VOLUME /tmp -ADD http://source.file/package.file.tar.gz /temp -RUN tar -xjf /temp/package.file.tar.gz \ - && make -C /tmp/package.file \ - && rm /tmp/ package.file.tar.gz -ARG JAR_FILE -ADD ${JAR_FILE} apps.jar -`, - }, - }, - { - expected: vulnerabilityLines{ - line: 6, - vulnLine: model.VulnLines{ - Positions: []int{5, 6, 7}, - Lines: []string{ - ` && apk add kubectl=1.20.0-r0 \`, - " && rm -rf /var/cache/apk/*", - `ENTRYPOINT ["kubectl"]`, - }, - }, - }, - searchKey: "FROM={{alpine:3.7}}.ENTRYPOINT[kubectl]", - ctx: &QueryContext{ - scanID: "Test", - }, - file: &model.FileMetadata{ - ScanID: "Test", - ID: "Test", - Kind: model.KindDOCKER, - OriginalData: `FROM alpine:3.7 -RUN apk update \ - && apk upgrade \ - && apk add kubectl=1.20.0-r0 \ - && rm -rf /var/cache/apk/* -ENTRYPOINT ["kubectl"]`, - }, - }, - } - - for i, testCase := range testCases { - t.Run(fmt.Sprintf("detectDockerLine-%d", i), func(t *testing.T) { - v := detectDockerLine(testCase.file, testCase.searchKey, &zerolog.Logger{}, 3) - require.Equal(t, testCase.expected, v) - }) - } -} - -// TestSelectLineWithMinimumDistance tests the functions [SelectLineWithMinimumDistance()] and all the methods called by them -func TestSelectLineWithMinimumDistance(t *testing.T) { - values := []struct { - distances map[int]int - startingFrom int - expectedResult int - }{ - { - distances: map[int]int{ - 12: 0, - }, - startingFrom: 0, - expectedResult: 12, - }, - { - distances: map[int]int{ - 12: 0, - 24: 0, - }, - startingFrom: 11, - expectedResult: 12, - }, - { - distances: map[int]int{ - 1: 26, - 2: 5, - 3: 0, - }, - startingFrom: 1, - expectedResult: 3, - }, - } - - for i, testCase := range values { - t.Run(fmt.Sprintf("selectLineWithMinimumDistance-%d", i), func(t *testing.T) { - v := selectLineWithMinimumDistance(testCase.distances, testCase.startingFrom) - require.Equal(t, testCase.expectedResult, v) - }) - } -} - // TestMapKeyToString tests the functions [MapKeyToString()] and all the methods called by them func TestMapKeyToString(t *testing.T) { testCases := []struct { @@ -267,135 +104,6 @@ func Test_mergeWithMetadata(t *testing.T) { } } -// Test_detectLine tests the functions [detectLine()] and all the methods called by them -func Test_detectLine(t *testing.T) { //nolint - type args struct { - ctx *QueryContext - file *model.FileMetadata - searchKey string - } - tests := []struct { - name string - args args - want vulnerabilityLines - }{ - { - name: "detect_line", - args: args{ - ctx: &QueryContext{ - scanID: "scanID", - }, - file: &model.FileMetadata{ - ScanID: "scanID", - ID: "Test", - Kind: model.KindTerraform, - OriginalData: `resource "aws_s3_bucket" "b" { - bucket = "my-tf-test-bucket" - acl = "authenticated-read" - - tags = { - Name = "My bucket" - Environment = "Dev" - } - } - `, - }, - searchKey: "aws_s3_bucket[b].acl", - }, - want: vulnerabilityLines{ - line: 3, - vulnLine: model.VulnLines{ - Positions: []int{2, 3, 4}, - Lines: []string{ - ` bucket = "my-tf-test-bucket"`, - ` acl = "authenticated-read"`, - "", - }, - }, - lineWithVulnerabilty: "\t\t\t\t\t\tacl = \"authenticated-read\"", - }, - }, - { - name: "detect_line_with_curly_brackets", - args: args{ - ctx: &QueryContext{ - scanID: "scanID", - }, - file: &model.FileMetadata{ - ScanID: "scanID", - ID: "Test", - Kind: model.KindTerraform, - OriginalData: `resource "aws_s3_bucket" "b" { - bucket = "my-tf-test-bucket" - acl = "authenticated-read" - - tags = { - Name = "My bucket" - Environment = "Dev.123" - Environment = "test" - } - } - `, - }, - searchKey: "aws_s3_bucket[b].Environment={{Dev.123}}", - }, - want: vulnerabilityLines{ - line: 7, - vulnLine: model.VulnLines{ - Positions: []int{6, 7, 8}, - Lines: []string{ - ` Name = "My bucket"`, - ` Environment = "Dev.123"`, - ` Environment = "test"`, - }, - }, - lineWithVulnerabilty: "\t\t\t\t\t\t Environment = \"Dev.123\"", - }, - }, - { - name: "detect_line_error", - args: args{ - ctx: &QueryContext{ - scanID: "scanID", - }, - file: &model.FileMetadata{ - ScanID: "scanID", - ID: "Test", - Kind: model.KindTerraform, - OriginalData: `resource "aws_s3_bucket" "b" { - bucket = "my-tf-test-bucket" - acl = "authenticated-read" - - tags = { - Name = "My bucket" - Environment = "Dev.123" - Environment = "test" - } - } - `, - }, - searchKey: "testing.error", - }, - want: vulnerabilityLines{ - line: -1, - vulnLine: model.VulnLines{}, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := detectLine(tt.args.file, tt.args.searchKey, &zerolog.Logger{}, 3) - gotStrVulnerabilities, err := test.StringifyStruct(got) - require.Nil(t, err) - wantStrVulnerabilities, err := test.StringifyStruct(tt.want) - require.Nil(t, err) - if !reflect.DeepEqual(gotStrVulnerabilities, wantStrVulnerabilities) { - t.Errorf("detectLine() = %v, want %v", gotStrVulnerabilities, wantStrVulnerabilities) - } - }) - } -} - // Test_mustMapKeyToString tests the functions [mustMapKeyToString()] and all the methods called by them func Test_mustMapKeyToString(t *testing.T) { type args struct { @@ -515,6 +223,7 @@ func TestDefaultVulnerabilityBuilder(t *testing.T) { QueryURI: "https://github.com/Checkmarx/kics/", Severity: model.SeverityInfo, Line: -1, + VulnLines: []model.CodeLine{}, IssueType: "IncorrectValue", SearchKey: "testSearchKey", KeyActualValue: "", @@ -527,8 +236,9 @@ func TestDefaultVulnerabilityBuilder(t *testing.T) { } for _, tt := range tests { + insDetector := detector.NewDetectLine(3) t.Run(tt.name, func(t *testing.T) { - got, err := DefaultVulnerabilityBuilder(tt.args.ctx, tt.args.tracker, tt.args.v) + got, err := DefaultVulnerabilityBuilder(tt.args.ctx, tt.args.tracker, tt.args.v, insDetector) if (err != nil) != tt.wantErr { t.Errorf("DefaultVulnerabilityBuilder() error = %v, wantErr %v", err, tt.wantErr) return @@ -539,427 +249,3 @@ func TestDefaultVulnerabilityBuilder(t *testing.T) { }) } } - -// TestGetBracketValues tests the functions [getBracketValues()] and all the methods called by them -func TestGetBracketValues(t *testing.T) { - type args struct { - expr string - } - tests := []struct { - name string - args args - want [][]string - }{ - { - name: "no_brackets", - args: args{ - expr: "password", - }, - want: [][]string{ - { - "{{password}}", - "password", - }, - }, - }, - { - name: "single_brackets", - args: args{ - expr: "{{password}}", - }, - want: [][]string{ - { - "{{password}}", - "password", - }, - }, - }, - { - name: "double_brackets", - args: args{ - expr: "{{ {{password}} }}", - }, - want: [][]string{ - { - "{{ {{password}}}}", - " {{password}}", - }, - }, - }, - { - name: "multiple_brackets", - args: args{ - expr: "FROM={{open-jdk}}.{{ {{password}} }}", - }, - want: [][]string{ - { - "{{open-jdk}}", - "open-jdk", - }, - { - "{{ {{password}}}}", - " {{password}}", - }, - }, - }, - } - - for _, tt := range tests { - var got [][]string - t.Run(tt.name, func(t *testing.T) { - got = getBracketValues(tt.args.expr, got, "") - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("DefaultVulnerabilityBuilder() = %v, want %v", got, tt.want) - } - }) - } -} - -// TestGetAdjacents tests the functions [GetAdjacents()] and all the methods called by them -func TestGetAdjacents(t *testing.T) { //nolint - type args struct { - idx int - adj int - lines []string - } - tests := []struct { - name string - args args - want model.VulnLines - }{ - { - name: "test_start_of_file", - args: args{ - idx: 0, - adj: 3, - lines: []string{ - "firstline", - "secondline", - "thirdline", - "forthline", - }, - }, - want: model.VulnLines{ - Positions: []int{1, 2, 3}, - Lines: []string{ - "firstline", - "secondline", - "thirdline", - }, - }, - }, - { - name: "test_end_of_file", - args: args{ - idx: 3, - adj: 3, - lines: []string{ - "firstline", - "secondline", - "thirdline", - "forthline", - }, - }, - want: model.VulnLines{ - Positions: []int{3, 4, 5}, - Lines: []string{ - "secondline", - "thirdline", - "forthline", - }, - }, - }, - { - name: "test_midle_of_file", - args: args{ - idx: 1, - adj: 3, - lines: []string{ - "firstline", - "secondline", - "thirdline", - "forthline", - }, - }, - want: model.VulnLines{ - Positions: []int{1, 2, 3}, - Lines: []string{ - "firstline", - "secondline", - "thirdline", - }, - }, - }, - { - name: "test_even_adj", - args: args{ - idx: 1, - adj: 2, - lines: []string{ - "firstline", - "secondline", - "thirdline", - "forthline", - }, - }, - want: model.VulnLines{ - Positions: []int{2, 3}, - Lines: []string{ - "secondline", - "thirdline", - }, - }, - }, - { - name: "test_even_adj_first_line", - args: args{ - idx: 0, - adj: 2, - lines: []string{ - "firstline", - "secondline", - "thirdline", - "forthline", - }, - }, - want: model.VulnLines{ - Positions: []int{1, 2}, - Lines: []string{ - "firstline", - "secondline", - }, - }, - }, - { - name: "test_one_adj", - args: args{ - idx: 3, - adj: 1, - lines: []string{ - "firstline", - "secondline", - "thirdline", - "forthline", - }, - }, - want: model.VulnLines{ - Positions: []int{4}, - Lines: []string{ - "forthline", - }, - }, - }, - { - name: "test_adj_bigger_than_file", - args: args{ - idx: 3, - adj: 5, - lines: []string{ - "firstline", - "secondline", - "thirdline", - "forthline", - }, - }, - want: model.VulnLines{ - Positions: []int{1, 2, 3, 4}, - Lines: []string{ - "firstline", - "secondline", - "thirdline", - "forthline", - }, - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := getAdjacentLines(tt.args.idx, tt.args.adj, tt.args.lines) - gotStrVulnerabilities, err := test.StringifyStruct(got) - require.Nil(t, err) - wantStrVulnerabilities, err := test.StringifyStruct(tt.want) - require.Nil(t, err) - if !reflect.DeepEqual(gotStrVulnerabilities, wantStrVulnerabilities) { - t.Errorf("getAdjacents() = %v, want = %v", gotStrVulnerabilities, wantStrVulnerabilities) - } - }) - } -} - -func TestEngine_detectHelmLine(t *testing.T) { //nolint - type args struct { - file *model.FileMetadata - searchKey string - logWithFields *zerolog.Logger - outputLines int - } - - tests := []struct { - name string - args args - want vulnerabilityLines - }{ - { - name: "test_detect_helm_line", - args: args{ - file: &model.FileMetadata{ - ID: "1", - ScanID: "console", - Document: model.Document{}, - Kind: model.KindHELM, - FileName: "test-connection.yaml", - HelmID: "# KICS_HELM_ID_0", - OriginalData: `# KICS_HELM_ID_0: -apiVersion: v1 -kind: Pod -metadata: - name: "{{ include "test_helm.fullname" . }}-test-connection" - labels: - {{- include "test_helm.labels" . | nindent 4 }} - annotations: - "helm.sh/hook": test -spec: - containers: - - name: wget - image: busybox - command: ['wget'] - args: ['{{ include "test_helm.fullname" . }}:{{ .Values.service.port }}'] - restartPolicy: Never -`, - Content: ``, - }, - searchKey: "KICS_HELM_ID_0.metadata.name={{RELEASE-NAME-test_helm-test-connection}}.spec.containers", - logWithFields: &zerolog.Logger{}, - outputLines: 1, - }, - want: vulnerabilityLines{ - line: 10, - vulnLine: model.VulnLines{ - Positions: []int{10}, - Lines: []string{" containers:"}, - }, - lineWithVulnerabilty: " containers:", - }, - }, - { - name: "test_dup_values", - args: args{ - file: &model.FileMetadata{ - ID: "1", - ScanID: "console", - Document: model.Document{}, - Kind: model.KindHELM, - FileName: "test-dup_values.yaml", - IDInfo: map[int]interface{}{0: map[int]int{0: 0, 1: 1, 2: 2, 3: 3, 4: 4, - 5: 5, 6: 6, 7: 7, 8: 8, 9: 9, 10: 10, 11: 11, 12: 12, 13: 13, 14: 14, 15: 15, 16: 16, 17: 17, - 18: 18, 19: 19, 21: 21, 22: 22}}, - HelmID: "# KICS_HELM_ID_0", - OriginalData: `# KICS_HELM_ID_0: -apiVersion: v1 -kind: Pod -metadata: - name: "{{ include "test_helm.fullname" . }}-test-connection" - labels: - {{- include "test_helm.labels" . | nindent 4 }} - annotations: - "helm.sh/hook": test -spec: - containers: - - name: wget - image: busybox - command: ['wget'] - args: ['{{ include "test_helm.fullname" . }}:{{ .Values.service.port }}'] - restartPolicy: Never - containers: - - name: wget2 - image: busybox - command: ['wget'] - args: ['{{ include "test_helm.fullname" . }}:{{ .Values.service.port }}'] - restartPolicy: Never -`, - Content: ``, - }, - searchKey: "KICS_HELM_ID_0.metadata.name={{RELEASE-NAME-test_helm-test-connection}}.spec.containers", - logWithFields: &zerolog.Logger{}, - outputLines: 1, - }, - want: vulnerabilityLines{ - line: 9, - vulnLine: model.VulnLines{ - Positions: []int{9}, - Lines: []string{"spec:"}, - }, - lineWithVulnerabilty: "spec:", - }, - }, - { - name: "test_detect_helm_with_dups", - args: args{ - file: &model.FileMetadata{ - ID: "1", - ScanID: "console", - Document: model.Document{}, - Kind: model.KindHELM, - FileName: "test-dups.yaml", - HelmID: "# KICS_HELM_ID_1", - OriginalData: `# KICS_HELM_ID_0: -apiVersion: v1 -kind: Pod -metadata: - name: "{{ include "test_helm.fullname" . }}-test-connection" - labels: - {{- include "test_helm.labels" . | nindent 4 }} - annotations: - "helm.sh/hook": test -spec: - containers: - - name: wget - image: busybox - command: ['wget'] - args: ['{{ include "test_helm.fullname" . }}:{{ .Values.service.port }}'] - restartPolicy: Never ---- -# KICS_HELM_ID_1: -apiVersion: v1 -kind: Pod -metadata: - name: "{{ include "test_helm.fullname" . }}-test-dups" - labels: - {{- include "test_helm.labels" . | nindent 4 }} - annotations: - "helm.sh/hook": test -spec: - containers: - - name: wget - image: busybox - command: ['wget'] - args: ['{{ include "test_helm.fullname" . }}:{{ .Values.service.port }}'] - restartPolicy: Never -`, - Content: ``, - }, - searchKey: "KICS_HELM_ID_1.metadata.name={{RELEASE-NAME-test_helm-test-connection}}.spec.containers", - logWithFields: &zerolog.Logger{}, - outputLines: 1, - }, - want: vulnerabilityLines{ - line: 26, - vulnLine: model.VulnLines{ - Positions: []int{26}, - Lines: []string{" containers:"}, - }, - lineWithVulnerabilty: " containers:", - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := detectHelmLine(tt.args.file, tt.args.searchKey, tt.args.logWithFields, tt.args.outputLines) - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("detectHelmLine() = %v, want = %v", got, tt.want) - } - }) - } -} diff --git a/pkg/kics/resolver_sink.go b/pkg/kics/resolver_sink.go new file mode 100644 index 00000000000..aac55228396 --- /dev/null +++ b/pkg/kics/resolver_sink.go @@ -0,0 +1,52 @@ +package kics + +import ( + "context" + "encoding/json" + + "github.com/Checkmarx/kics/pkg/model" + "github.com/getsentry/sentry-go" + "github.com/google/uuid" + "github.com/pkg/errors" + "github.com/rs/zerolog/log" +) + +func (s *Service) resolverSink(ctx context.Context, filename, scanID string) error { + s.Tracker.TrackFileFound() + kind := s.Resolver.GetType(filename) + if kind == model.KindCOMMON { + return nil + } + resFiles, err := s.Resolver.Resolve(filename, kind) + if err != nil { + return errors.Wrap(err, "failed to render file content") + } + for _, rfile := range resFiles.File { + documents, _, err := s.Parser.Parse(rfile.FileName, rfile.Content) + if err != nil { + return errors.Wrap(err, "failed to parse file content") + } + for _, document := range documents { + _, err = json.Marshal(document) + if err != nil { + sentry.CaptureException(err) + log.Err(err).Msgf("failed to marshal content in file: %s", rfile.FileName) + continue + } + + file := model.FileMetadata{ + ID: uuid.New().String(), + ScanID: scanID, + Document: document, + OriginalData: string(rfile.OriginalData), + Kind: kind, + FileName: rfile.FileName, + Content: string(rfile.Content), + HelmID: rfile.SplitID, + IDInfo: rfile.IDInfo, + } + s.saveToFile(ctx, &file) + } + } + return nil +} diff --git a/pkg/kics/service.go b/pkg/kics/service.go index 21e9d13d434..b7fac6639c3 100644 --- a/pkg/kics/service.go +++ b/pkg/kics/service.go @@ -2,7 +2,6 @@ package kics import ( "context" - "encoding/json" "io" "github.com/Checkmarx/kics/pkg/engine" @@ -10,8 +9,6 @@ import ( "github.com/Checkmarx/kics/pkg/model" "github.com/Checkmarx/kics/pkg/parser" "github.com/Checkmarx/kics/pkg/resolver" - "github.com/getsentry/sentry-go" - "github.com/google/uuid" "github.com/pkg/errors" "github.com/rs/zerolog/log" ) @@ -46,92 +43,26 @@ type Service struct { Inspector *engine.Inspector Tracker Tracker Resolver *resolver.Resolver + files model.FileMetadatas } // StartScan executes scan over the context, using the scanID as reference func (s *Service) StartScan(ctx context.Context, scanID string, hideProgress bool) error { log.Debug().Msg("service.StartScan()") - var files model.FileMetadatas if err := s.SourceProvider.GetSources( ctx, s.Parser.SupportedExtensions(), func(ctx context.Context, filename string, rc io.ReadCloser) error { - s.Tracker.TrackFileFound() - - content, err := getContent(rc) - if err != nil { - return errors.Wrapf(err, "failed to get file content: %s", filename) - } - - documents, kind, err := s.Parser.Parse(filename, *content) - if err != nil { - return errors.Wrap(err, "failed to parse file content") - } - for _, document := range documents { - _, err = json.Marshal(document) - if err != nil { - sentry.CaptureException(err) - log.Err(err).Msgf("failed to marshal content in file: %s", filename) - continue - } - - file := model.FileMetadata{ - ID: uuid.New().String(), - ScanID: scanID, - Document: document, - OriginalData: string(*content), - Kind: kind, - FileName: filename, - } - files = s.saveToFile(ctx, &file, files) - } - - return errors.Wrap(err, "failed to save file content") + return s.sink(ctx, filename, scanID, rc) }, func(ctx context.Context, filename string) error { // Sink used for resolver files and templates - s.Tracker.TrackFileFound() - kind := s.Resolver.GetType(filename) - if kind == model.KindCOMMON { - return nil - } - resFiles, err := s.Resolver.Resolve(filename, kind) - if err != nil { - return errors.Wrap(err, "failed to render file content") - } - for _, rfile := range resFiles.File { - documents, _, err := s.Parser.Parse(rfile.FileName, rfile.Content) - if err != nil { - return errors.Wrap(err, "failed to parse file content") - } - for _, document := range documents { - _, err = json.Marshal(document) - if err != nil { - sentry.CaptureException(err) - log.Err(err).Msgf("failed to marshal content in file: %s", rfile.FileName) - continue - } - - file := model.FileMetadata{ - ID: uuid.New().String(), - ScanID: scanID, - Document: document, - OriginalData: string(rfile.OriginalData), - Kind: kind, - FileName: rfile.FileName, - Content: string(rfile.Content), - HelmID: rfile.SplitID, - IDInfo: rfile.IDInfo, - } - files = s.saveToFile(ctx, &file, files) - } - } - return nil + return s.resolverSink(ctx, filename, scanID) }, ); err != nil { return errors.Wrap(err, "failed to read sources") } - vulnerabilities, err := s.Inspector.Inspect(ctx, scanID, files, hideProgress, s.SourceProvider.GetBasePath()) + vulnerabilities, err := s.Inspector.Inspect(ctx, scanID, s.files, hideProgress, s.SourceProvider.GetBasePath()) if err != nil { return errors.Wrap(err, "failed to inspect files") } @@ -177,11 +108,10 @@ func (s *Service) GetScanSummary(ctx context.Context, scanIDs []string) ([]model return s.Storage.GetScanSummary(ctx, scanIDs) } -func (s *Service) saveToFile(ctx context.Context, file *model.FileMetadata, files model.FileMetadatas) model.FileMetadatas { +func (s *Service) saveToFile(ctx context.Context, file *model.FileMetadata) { err := s.Storage.SaveFile(ctx, file) if err == nil { - files = append(files, *file) + s.files = append(s.files, *file) s.Tracker.TrackFileParse() } - return files } diff --git a/pkg/kics/service_test.go b/pkg/kics/service_test.go index dd229904448..b25a9ee1013 100644 --- a/pkg/kics/service_test.go +++ b/pkg/kics/service_test.go @@ -16,11 +16,13 @@ import ( jsonParser "github.com/Checkmarx/kics/pkg/parser/json" terraformParser "github.com/Checkmarx/kics/pkg/parser/terraform" yamlParser "github.com/Checkmarx/kics/pkg/parser/yaml" + "github.com/Checkmarx/kics/pkg/resolver" + "github.com/Checkmarx/kics/pkg/resolver/helm" ) // TestService tests the functions [GetVulnerabilities(), GetScanSummary(),StartScan()] and all the methods called by them func TestService(t *testing.T) { - mockParser, mockFilesSource := createParserSourceProvider("../../assets/queries/template") + mockParser, mockFilesSource, mockResolver := createParserSourceProvider("../../test/fixtures/test_helm") type fields struct { SourceProvider provider.SourceProvider @@ -28,6 +30,7 @@ func TestService(t *testing.T) { Parser *parser.Parser Inspector *engine.Inspector Tracker Tracker + Resolver *resolver.Resolver } type args struct { ctx context.Context @@ -53,6 +56,7 @@ func TestService(t *testing.T) { Tracker: &tracker.CITracker{}, Storage: storage.NewMemoryStorage(), SourceProvider: mockFilesSource, + Resolver: mockResolver, }, args: args{ ctx: nil, @@ -73,6 +77,7 @@ func TestService(t *testing.T) { Parser: tt.fields.Parser, Inspector: tt.fields.Inspector, Tracker: tt.fields.Tracker, + Resolver: tt.fields.Resolver, } t.Run(fmt.Sprintf(tt.name+"_get_vulnerabilities"), func(t *testing.T) { got, err := s.GetVulnerabilities(tt.args.ctx, tt.args.scanID) @@ -102,7 +107,8 @@ func TestService(t *testing.T) { } } -func createParserSourceProvider(path string) (*parser.Parser, *provider.FileSystemSourceProvider) { +func createParserSourceProvider(path string) (*parser.Parser, + *provider.FileSystemSourceProvider, *resolver.Resolver) { mockParser, _ := parser.NewBuilder(). Add(&jsonParser.Parser{}). Add(&yamlParser.Parser{}). @@ -112,5 +118,7 @@ func createParserSourceProvider(path string) (*parser.Parser, *provider.FileSyst mockFilesSource, _ := provider.NewFileSystemSourceProvider(path, []string{}) - return mockParser, mockFilesSource + mockResolver, _ := resolver.NewBuilder().Add(&helm.Resolver{}).Build() + + return mockParser, mockFilesSource, mockResolver } diff --git a/pkg/kics/sink.go b/pkg/kics/sink.go new file mode 100644 index 00000000000..76cb37d020a --- /dev/null +++ b/pkg/kics/sink.go @@ -0,0 +1,47 @@ +package kics + +import ( + "context" + "encoding/json" + "io" + + "github.com/Checkmarx/kics/pkg/model" + "github.com/getsentry/sentry-go" + "github.com/google/uuid" + "github.com/pkg/errors" + "github.com/rs/zerolog/log" +) + +func (s *Service) sink(ctx context.Context, filename, scanID string, rc io.Reader) error { + s.Tracker.TrackFileFound() + + content, err := getContent(rc) + if err != nil { + return errors.Wrapf(err, "failed to get file content: %s", filename) + } + + documents, kind, err := s.Parser.Parse(filename, *content) + if err != nil { + return errors.Wrap(err, "failed to parse file content") + } + for _, document := range documents { + _, err = json.Marshal(document) + if err != nil { + sentry.CaptureException(err) + log.Err(err).Msgf("failed to marshal content in file: %s", filename) + continue + } + + file := model.FileMetadata{ + ID: uuid.New().String(), + ScanID: scanID, + Document: document, + OriginalData: string(*content), + Kind: kind, + FileName: filename, + } + s.saveToFile(ctx, &file) + } + + return errors.Wrap(err, "failed to save file content") +} diff --git a/pkg/model/model.go b/pkg/model/model.go index 21ac11d5220..3c388a420a9 100644 --- a/pkg/model/model.go +++ b/pkg/model/model.go @@ -49,6 +49,13 @@ var ( } ) +// VulnerabilityLines is the representation of the found line for issue +type VulnerabilityLines struct { + Line int + VulnLines []CodeLine + LineWithVulnerabilty string +} + // FileKind is the extension of a file type FileKind string @@ -58,10 +65,10 @@ type Severity string // IssueType is the issue's type string representation type IssueType string -// VulnLines is the lines containing and adjecent to the vulnerability line with their respective positions -type VulnLines struct { - Positions []int - Lines []string +// CodeLine is the lines containing and adjecent to the vulnerability line with their respective positions +type CodeLine struct { + Position int + Line string } // FileMetadata is a representation of basic information and content of a file @@ -91,27 +98,27 @@ type QueryMetadata struct { // Vulnerability is a representation of a detected vulnerability in scanned files // after running a query type Vulnerability struct { - ID int `json:"id"` - ScanID string `db:"scan_id" json:"-"` - SimilarityID string `db:"similarity_id" json:"similarityID"` - FileID string `db:"file_id" json:"-"` - FileName string `db:"file_name" json:"fileName"` - QueryID string `db:"query_id" json:"queryID"` - QueryName string `db:"query_name" json:"queryName"` - QueryURI string `json:"-"` - Category string `json:"category"` - Description string `json:"description"` - Platform string `db:"platform" json:"platform"` - Severity Severity `json:"severity"` - Line int `json:"line"` - VulnLines VulnLines `json:"vulnLines"` - IssueType IssueType `db:"issue_type" json:"issueType"` - SearchKey string `db:"search_key" json:"searchKey"` - SearchValue string `db:"search_value" json:"searchValue"` - KeyExpectedValue string `db:"key_expected_value" json:"expectedValue"` - KeyActualValue string `db:"key_actual_value" json:"actualValue"` - Value *string `db:"value" json:"value"` - Output string `json:"-"` + ID int `json:"id"` + ScanID string `db:"scan_id" json:"-"` + SimilarityID string `db:"similarity_id" json:"similarityID"` + FileID string `db:"file_id" json:"-"` + FileName string `db:"file_name" json:"fileName"` + QueryID string `db:"query_id" json:"queryID"` + QueryName string `db:"query_name" json:"queryName"` + QueryURI string `json:"-"` + Category string `json:"category"` + Description string `json:"description"` + Platform string `db:"platform" json:"platform"` + Severity Severity `json:"severity"` + Line int `json:"line"` + VulnLines []CodeLine `json:"vulnLines"` + IssueType IssueType `db:"issue_type" json:"issueType"` + SearchKey string `db:"search_key" json:"searchKey"` + SearchValue string `db:"search_value" json:"searchValue"` + KeyExpectedValue string `db:"key_expected_value" json:"expectedValue"` + KeyActualValue string `db:"key_actual_value" json:"actualValue"` + Value *string `db:"value" json:"value"` + Output string `json:"-"` } // QueryConfig is a struct that contains the fileKind and platform of the rego query diff --git a/pkg/model/summary.go b/pkg/model/summary.go index c7925815404..b0a0c4c02b9 100644 --- a/pkg/model/summary.go +++ b/pkg/model/summary.go @@ -15,16 +15,16 @@ type SeveritySummary struct { // VulnerableFile contains information of a vulnerable file and where the vulnerability was found type VulnerableFile struct { - FileName string `json:"file_name"` - SimilarityID string `json:"similarity_id"` - Line int `json:"line"` - VulnLines VulnLines `json:"-"` - IssueType IssueType `json:"issue_type"` - SearchKey string `json:"search_key"` - SearchValue string `json:"search_value"` - KeyExpectedValue string `json:"expected_value"` - KeyActualValue string `json:"actual_value"` - Value *string `json:"value"` + FileName string `json:"file_name"` + SimilarityID string `json:"similarity_id"` + Line int `json:"line"` + VulnLines []CodeLine `json:"-"` + IssueType IssueType `json:"issue_type"` + SearchKey string `json:"search_key"` + SearchValue string `json:"search_value"` + KeyExpectedValue string `json:"expected_value"` + KeyActualValue string `json:"actual_value"` + Value *string `json:"value"` } // VulnerableQuery contains a query that tested positive ID, name, severity and a list of files that tested vulnerable diff --git a/pkg/report/template/html/report.tmpl b/pkg/report/template/html/report.tmpl index 6d8b88337cc..56faca5014f 100644 --- a/pkg/report/template/html/report.tmpl +++ b/pkg/report/template/html/report.tmpl @@ -76,15 +76,10 @@ Found: {{ .KeyActualValue }}
- {{- with .VulnLines -}} - {{- $lines := .Lines -}} - {{- range $idx, $position := .Positions -}} -
- {{- if lt $idx (len $lines) -}} - {{ $position }}{{index $lines $idx | trimSpaces }} - {{- end -}} + {{- range .VulnLines -}} +
+ {{ .Position }}{{ trimSpaces .Line }}
- {{- end -}} {{- end}}
diff --git a/pkg/resolver/helm/resolver.go b/pkg/resolver/helm/resolver.go index d59f88f24ac..cb5b17e79fe 100644 --- a/pkg/resolver/helm/resolver.go +++ b/pkg/resolver/helm/resolver.go @@ -123,7 +123,7 @@ func updateName(template []*chart.File, charts *chart.Chart, name string) []*cha } // getIdMap will construct a map with ids with the corresponding lines as keys -// for use in detectline +// for use in detector func getIDMap(originalData []byte) (map[int]interface{}, error) { ids := make(map[int]interface{}) mapLines := make(map[int]int) diff --git a/test/queries_content_test.go b/test/queries_content_test.go index 33c3fa63613..23b7ee7ba3a 100644 --- a/test/queries_content_test.go +++ b/test/queries_content_test.go @@ -13,6 +13,7 @@ import ( "testing" "github.com/Checkmarx/kics/internal/tracker" + "github.com/Checkmarx/kics/pkg/detector" "github.com/Checkmarx/kics/pkg/engine" "github.com/Checkmarx/kics/pkg/engine/mock" "github.com/Checkmarx/kics/pkg/engine/source" @@ -185,7 +186,7 @@ func testQueryHasGoodReturnParams(t *testing.T, entry queryEntry) { inspector, err := engine.NewInspector( ctx, queriesSource, - func(ctx *engine.QueryContext, trk engine.Tracker, v interface{}) (model.Vulnerability, error) { + func(ctx *engine.QueryContext, trk engine.Tracker, v interface{}, detector *detector.DetectLine) (model.Vulnerability, error) { m, ok := v.(map[string]interface{}) require.True(t, ok)