horusec:fix - Errors reported in v2.8.0-beta.1 (#1050)

In this commit I made some changes to the code to improve the
identification and generation of vulnerabilities pointed out by Horusec.
* Now when Horusec identifies that there are duplicate hashes in its
analysis by the same tool, all vulnerability descriptions will be grouped
by the `(x/x) separator * Possible vulnerability detected:` demonstrating
the amount of vulnerabilities that hash generated.
* The `Details` field will be the last to be shown in each problem
reported by Horusec in order to improve the experience and identification.
* Tools like `DotnetCLI, BundlerAudit, Trivy, Safety, Nancy` were pointing
out multiple vulnerabilities with the same hash because they couldn't find
the exact line that contains the vulnerability. So an improvement has been
implemented where using the `file.GetDependencyInfo` method will be a
better way to identify the vulnerability
* The `Trivy` tool was reporting problems finding the exact line so we
noticed that when running the analysis on infrastructure configuration
files the tool returns the line that has the problem,
so now it can be more assertive with this improvement.
* The `BundlerAudit` tool was quite complex in identifying vulnerabilities
and with complex treatments, so we made an improvement so that the tool's
output is in json format,
so we will have better control of the information shown.
* Tool versions update
  * horuszup/horusec-generic updated to v1.2.0
    * semgrep updated to v0.85.0 version
    * owasp-dependency-check updated to v6.5.3
    * updated trivy to v0.24.4 version
  * horuszup/horusec-go updated to v1.3.0
    * nancy updated to version v1.0.33
    * gosec updated to v2.11.0 version
  * horuszup/horusec-python updated to v1.0.1 version
    * updated bandit to v1.7.4 version
  * horuszup/horusec-ruby updated to v1.2.0
    * Ruby updated to v3.1-alpine version
* The e2e tests broke due to the joining of the hashes so now they are more
assertive and I made an improvement in the test of the `Gitleaks` tool
because validating that the tool was not running was not a good practice.
But to run e2e tests in the `../horusec-examples-vulnerabilities` directory
there must be [our test repository](https://github.com/ZupIT/horusec-examples-vulnerabilities).

Signed-off-by: Wilian Gabriel <[email protected]>
(cherry picked from commit 4ff44db)
Signed-off-by: Wilian Gabriel <[email protected]>

.github/workflows/e2e.yml

@@ -21,12 +21,11 @@ on:
   push:
     branches:
       - main
-permissions: read-all
+
+permissions:
+  contents: write
 jobs:
   test:
-    permissions:
-      contents: write
-      packages: write
     runs-on: ${{ matrix.os }}
     strategy:
       matrix:
@@ -49,9 +48,13 @@ jobs:
         with:
           go-version: ${{ matrix.go-version }}
 
+      - name: Download Examples repository
+        run: |
+          git clone https://github.com/ZupIT/horusec-examples-vulnerabilities.git ../horusec-examples-vulnerabilities
+
       - name: Setup docker for MacOS
         if: ${{ matrix.os == 'macos-latest' }}
-        uses: docker-practice/actions-setup-docker@master
+        uses: docker-practice/actions-setup-docker@v1
         with:
           docker_buildx: false
       - name: Build local images

.github/workflows/test.yml

@@ -46,7 +46,7 @@ jobs:
 
       - name: Setup docker for MacOS
         if: ${{ matrix.os == 'macos-latest' }} 
-        uses: docker-practice/actions-setup-docker@master
+        uses: docker-practice/actions-setup-docker@v1
         with:
           docker_buildx: false
 

e2e/analysis/test_case.go

@@ -16,6 +16,7 @@ package analysis
 
 import (
 	"fmt"
+	"path/filepath"
 	"runtime"
 
 	"github.com/ZupIT/horusec-devkit/pkg/enums/analysis"
@@ -67,7 +68,8 @@ func NewTestCase() []*TestCase {
 			RequiredDocker: false,
 			Command: Command{
 				Flags: map[string]string{
-					testutil.StartFlagProjectPath: testutil.ExamplesPath,
+					testutil.StartFlagProjectPath:         testutil.ExamplesPath,
+					testutil.StartFlagInformationSeverity: "true",
 				},
 			},
 			Expected: Expected{
@@ -77,11 +79,12 @@ func NewTestCase() []*TestCase {
 					fmt.Sprintf(messages.MsgPrintFinishAnalysisWithStatus, analysis.Success),
 					messages.MsgDebugVulnHashToFix,
 					messages.MsgWarnAnalysisFoundVulns[16:],
-					"In this analysis, a total of 61 possible vulnerabilities were found and we classified them into:",
+					"In this analysis, a total of 78 possible vulnerabilities were found and we classified them into:",
 					"Total of Vulnerability CRITICAL is: 22",
 					"Total of Vulnerability HIGH is: 24",
-					"Total of Vulnerability MEDIUM is: 12",
-					"Total of Vulnerability LOW is: 3",
+					"Total of Vulnerability MEDIUM is: 9",
+					"Total of Vulnerability LOW is: 2",
+					"Total of Vulnerability INFO is: 21",
 					fmt.Sprintf("{HORUSEC_CLI} Running %s - %s", tools.HorusecEngine, languages.CSharp),
 					fmt.Sprintf("{HORUSEC_CLI} Running %s - %s", tools.HorusecEngine, languages.Dart),
 					fmt.Sprintf("{HORUSEC_CLI} Running %s - %s", tools.HorusecEngine, languages.Java),
@@ -115,6 +118,7 @@ func NewTestCase() []*TestCase {
 					fmt.Sprintf("{HORUSEC_CLI} The tool was ignored for run in this analysis: %s", tools.DotnetCli),
 					fmt.Sprintf("{HORUSEC_CLI} The tool was ignored for run in this analysis: %s", tools.Nancy),
 					fmt.Sprintf("{HORUSEC_CLI} The tool was ignored for run in this analysis: %s", tools.Trivy),
+					`Details: (1/4) * Possible vulnerability detected:`,
 				},
 				OutputsNotContains: []string{
 					fmt.Sprintf("{HORUSEC_CLI} The tool was ignored for run in this analysis: %s", tools.HorusecEngine),
@@ -643,15 +647,15 @@ func NewTestCase() []*TestCase {
 			RequiredDocker: true,
 			Command: Command{
 				Flags: map[string]string{
-					testutil.StartFlagProjectPath:        testutil.ExamplesPath,
+					testutil.StartFlagProjectPath:        filepath.Join(testutil.RootPath, "..", "horusec-examples-vulnerabilities"),
 					testutil.StartFlagEnableGitHistory:   "true",
 					testutil.StartFlagEnableCommitAuthor: "true",
 					testutil.StartFlagAnalysisTimeout:    "10000",
 					testutil.StartFlagIgnore:             "**/ruby/**, **/javascript/**, **/python/**, **/go/**",
 				},
 			},
 			Expected: Expected{
-				Language: languages.Generic,
+				Language: languages.Leaks,
 				ExitCode: 0,
 				OutputsContains: []string{
 					fmt.Sprintf(messages.MsgPrintFinishAnalysisWithStatus, analysis.Success),
@@ -661,10 +665,10 @@ func NewTestCase() []*TestCase {
 					fmt.Sprintf("{HORUSEC_CLI} %s - %s is finished in analysisID:", tools.GitLeaks, languages.Leaks),
 					fmt.Sprintf("{HORUSEC_CLI} Running %s - %s", tools.HorusecEngine, languages.Leaks),
 					fmt.Sprintf("{HORUSEC_CLI} %s - %s is finished in analysisID:", tools.HorusecEngine, languages.Leaks),
-					fmt.Sprintf("{HORUSEC_CLI} The current path it's not a valid git repository"),
 				},
 				OutputsNotContains: []string{
 					fmt.Sprintf("{HORUSEC_CLI} Something error went wrong in %s tool", tools.GitLeaks),
+					fmt.Sprintf("{HORUSEC_CLI} Error while running tool %s: {HORUSEC_CLI} The current path it's not a valid git repository", tools.GitLeaks),
 				},
 			},
 		},

e2e/commands/start/start_test.go

@@ -479,9 +479,9 @@ var _ = Describe("running binary Horusec with start parameter", func() {
 			Eventually(session.Wait(testutil.AverageTimeoutAnalyzeForExamplesFolder).Out).Should(gbytes.Say(`Severity: LOW`))
 			Eventually(session.Wait(testutil.AverageTimeoutAnalyzeForExamplesFolder).Out).Should(gbytes.Say(`Confidence: LOW`))
 			Eventually(session.Wait(testutil.AverageTimeoutAnalyzeForExamplesFolder).Out).Should(gbytes.Say(`RuleID: HS-JAVA-99999999999`))
-			Eventually(session.Wait(testutil.AverageTimeoutAnalyzeForExamplesFolder).Out).Should(gbytes.Say(`Details: Teste QA`))
-			Eventually(session.Wait(testutil.AverageTimeoutAnalyzeForExamplesFolder).Out).Should(gbytes.Say(`Teste de description QA`))
 			Eventually(session.Wait(testutil.AverageTimeoutAnalyzeForExamplesFolder).Out).Should(gbytes.Say(`Type: Vulnerability`))
+			Eventually(session.Wait(testutil.AverageTimeoutAnalyzeForExamplesFolder).Out).Should(gbytes.Say(`Possible vulnerability detected: Test QA`))
+			Eventually(session.Wait(testutil.AverageTimeoutAnalyzeForExamplesFolder).Out).Should(gbytes.Say(`Test Description QA`))
 		})
 	})
 })
@@ -497,8 +497,8 @@ func writeJsonFile(path string) {
 	customRules := []map[string]interface{}{
 		{
 			"id":          "HS-JAVA-99999999999",
-			"name":        "Teste QA",
-			"description": "Teste de description QA",
+			"name":        "Test QA",
+			"description": "Test Description QA",
 			"language":    "Java",
 			"severity":    "LOW",
 			"confidence":  "LOW",

go.mod

@@ -29,7 +29,7 @@ require (
 	github.com/Microsoft/go-winio v0.5.1 // indirect
 	github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d // indirect
 	github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e // indirect
-	github.com/containerd/containerd v1.5.8 // indirect
+	github.com/containerd/containerd v1.5.10 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/docker/distribution v2.7.1+incompatible // indirect
 	github.com/docker/go-connections v0.4.0 // indirect

go.sum

@@ -202,8 +202,8 @@ github.com/containerd/containerd v1.5.0-beta.1/go.mod h1:5HfvG1V2FsKesEGQ17k5/T7
 github.com/containerd/containerd v1.5.0-beta.3/go.mod h1:/wr9AVtEM7x9c+n0+stptlo/uBBoBORwEx6ardVcmKU=
 github.com/containerd/containerd v1.5.0-beta.4/go.mod h1:GmdgZd2zA2GYIBZ0w09ZvgqEq8EfBp/m3lcVZIvPHhI=
 github.com/containerd/containerd v1.5.0-rc.0/go.mod h1:V/IXoMqNGgBlabz3tHD2TWDoTJseu1FGOKuoA4nNb2s=
-github.com/containerd/containerd v1.5.8 h1:NmkCC1/QxyZFBny8JogwLpOy2f+VEbO/f6bV2Mqtwuw=
-github.com/containerd/containerd v1.5.8/go.mod h1:YdFSv5bTFLpG2HIYmfqDpSYYTDX+mc5qtSuYx1YUb/s=
+github.com/containerd/containerd v1.5.10 h1:3cQ2uRVCkJVcx5VombsE7105Gl9Wrl7ORAO3+4+ogf4=
+github.com/containerd/containerd v1.5.10/go.mod h1:fvQqCfadDGga5HZyn3j4+dx56qj2I9YwBrlSdalvJYQ=
 github.com/containerd/continuity v0.0.0-20190426062206-aaeac12a7ffc/go.mod h1:GL3xCUCBDV3CZiTSEKksMWbLE66hEyuu9qyDOOqM47Y=
 github.com/containerd/continuity v0.0.0-20190815185530-f2a389ac0a02/go.mod h1:GL3xCUCBDV3CZiTSEKksMWbLE66hEyuu9qyDOOqM47Y=
 github.com/containerd/continuity v0.0.0-20191127005431-f65d91d395eb/go.mod h1:GL3xCUCBDV3CZiTSEKksMWbLE66hEyuu9qyDOOqM47Y=

internal/controllers/analyzer/analyzer.go

@@ -15,6 +15,7 @@
 package analyzer
 
 import (
+	"bytes"
 	"fmt"
 	"strings"
 	"time"
@@ -57,6 +58,8 @@ type HorusecService interface {
 	GetAnalysis(uuid.UUID) (*analysis.Analysis, error)
 }
 
+const detailsHeaderText = "* Possible vulnerability detected: "
+
 // Analyzer is responsible to orchestrate the pipeline of an analysis.
 //
 // Basically, an analysis has the following steps:
@@ -156,6 +159,7 @@ func (a *Analyzer) formatAnalysisToSendToAPI() {
 	a.analysis = a.setDefaultVulnerabilityType()
 	a.analysis = a.setDefaultConfidence()
 	a.analysis = a.sortVulnerabilitiesByType()
+	a.analysis = a.joinAllVulnerabilitiesOfSameToolAndHash()
 	if !a.config.EnableInformationSeverity {
 		a.analysis = a.removeInfoVulnerabilities()
 	}
@@ -295,6 +299,14 @@ func (a *Analyzer) sortVulnerabilitiesByType() *analysis.Analysis {
 	return a.analysis
 }
 
+// joinAllVulnerabilitiesOfSameToolAndHash will check if exists duplicated hash
+// and join the details in only one vulnerability
+func (a *Analyzer) joinAllVulnerabilitiesOfSameToolAndHash() *analysis.Analysis {
+	newAnalysisVulnerabilities := a.getAllVulnerabilitiesWithDetailsJoined()
+	a.analysis.AnalysisVulnerabilities = a.setCounterOfDetailsDuplicated(newAnalysisVulnerabilities)
+	return a.analysis
+}
+
 func (a *Analyzer) getVulnerabilitiesByType(
 	vulnType enumsVulnerability.Type,
 ) (response []analysis.AnalysisVulnerabilities) {
@@ -457,3 +469,60 @@ func (a *Analyzer) isWarning(err string) bool {
 		strings.Contains(err, messages.MsgWarnBrakemanNotRubyOnRailsProject) ||
 		strings.Contains(err, messages.MsgWarnGemfileIsRequiredForBundler)
 }
+
+// getAllVulnerabilitiesWithDetailsJoined will add the default separator of the details and will check if the hash
+// already exists in list of the vulnerabilities, if is duplicated, so, it will only join both details.
+// nolint:funlen,gocyclo // Breaking this function will make it more confusing
+func (a *Analyzer) getAllVulnerabilitiesWithDetailsJoined() (
+	newAnalysisVulnerabilities []analysis.AnalysisVulnerabilities,
+) {
+	for currentIndex := range a.analysis.AnalysisVulnerabilities {
+		currentAV := a.analysis.AnalysisVulnerabilities[currentIndex]
+		exists := false
+		for newIndex := range newAnalysisVulnerabilities {
+			newAV := newAnalysisVulnerabilities[newIndex]
+			if currentAV.Vulnerability.VulnHash == newAV.Vulnerability.VulnHash &&
+				!strings.Contains(newAV.Vulnerability.Details, currentAV.Vulnerability.Details) {
+				exists = true
+				newAnalysisVulnerabilities[newIndex].Vulnerability.Details = fmt.Sprintf("%s\n         %s%s",
+					newAV.Vulnerability.Details, detailsHeaderText, currentAV.Vulnerability.Details)
+				break
+			}
+		}
+		if !exists {
+			currentAV.Vulnerability.Details = fmt.Sprintf("%s%s", detailsHeaderText, currentAV.Vulnerability.Details)
+			newAnalysisVulnerabilities = append(newAnalysisVulnerabilities, currentAV)
+		}
+	}
+	return newAnalysisVulnerabilities
+}
+
+// setCounterOfDetailsDuplicated will check how many details there are by looking
+// for the detailsHeaderText separator constant and will update to add a counter of the details in this vulnerability.
+// nolint:funlen,gocyclo // Breaking this function will make it more confusing
+func (a *Analyzer) setCounterOfDetailsDuplicated(
+	analysisVulnerabilities []analysis.AnalysisVulnerabilities,
+) (newAnalysisVulnerabilities []analysis.AnalysisVulnerabilities) {
+	for indexAV := range analysisVulnerabilities {
+		currentAV := analysisVulnerabilities[indexAV]
+		newDetails := bytes.NewBufferString("")
+		detailEmptySplit := strings.Split(currentAV.Vulnerability.Details, detailsHeaderText)
+		detailToSetSplit := make([]string, 0)
+		for _, d := range detailEmptySplit {
+			if d != "" {
+				detailToSetSplit = append(detailToSetSplit, d)
+			}
+		}
+		for indexDS, detail := range detailToSetSplit {
+			lastChar := "\n"
+			if indexDS+1 == len(detailToSetSplit) {
+				lastChar = ""
+			}
+			_, _ = fmt.Fprintf(newDetails, "%s(%v/%v) %s%s%s", newDetails, indexDS+1, len(detailToSetSplit),
+				detailsHeaderText, detail, lastChar)
+		}
+		currentAV.Vulnerability.Details = newDetails.String()
+		newAnalysisVulnerabilities = append(newAnalysisVulnerabilities, currentAV)
+	}
+	return newAnalysisVulnerabilities
+}

internal/controllers/printresults/print_results.go

@@ -321,13 +321,14 @@ func (pr *PrintResults) printTextOutputVulnerabilityData(vulnerability *vulnerab
 	if vulnerability.RuleID != "" {
 		pr.printlnf("RuleID: %s", vulnerability.RuleID)
 	}
-	pr.printlnf("Details: %s", vulnerability.Details)
 	pr.printlnf("Type: %s", vulnerability.Type)
 
 	pr.printCommitAuthor(vulnerability)
 
 	pr.printlnf("ReferenceHash: %s", vulnerability.VulnHash)
 
+	pr.printlnf("Details: %s", vulnerability.Details)
+
 	pr.logSeparator(true)
 }
 

internal/enums/images/images.go

@@ -21,14 +21,14 @@ const (
 	C               = "horuszup/horusec-c:v1.0.1"
 	Csharp          = "horuszup/horusec-csharp:v1.2.0"
 	Elixir          = "horuszup/horusec-elixir:v1.1.0"
-	Generic         = "horuszup/horusec-generic:v1.1.0"
-	Go              = "horuszup/horusec-go:v1.2.1"
+	Generic         = "horuszup/horusec-generic:v1.2.0"
+	Go              = "horuszup/horusec-go:v1.3.0"
 	HCL             = "horuszup/horusec-hcl:v1.1.0"
 	Javascript      = "horuszup/horusec-js:v1.2.0"
 	Leaks           = "horuszup/horusec-leaks:v1.2.0"
 	PHP             = "horuszup/horusec-php:v1.0.1"
-	Python          = "horuszup/horusec-python:v1.0.0"
-	Ruby            = "horuszup/horusec-ruby:v1.1.1"
+	Python          = "horuszup/horusec-python:v1.0.1"
+	Ruby            = "horuszup/horusec-ruby:v1.2.0"
 	Shell           = "horuszup/horusec-shell:v1.0.1"
 )
 

internal/helpers/messages/error.go

@@ -21,6 +21,7 @@ const (
 	MsgErrorPathNotValid                        = "invalid path:"
 	MsgErrorJSONOutputFilePathNotValidExtension = "Output File path not valid file of type:"
 	MsgErrorJSONOutputFilePathNotValidUnknown   = "Output File path is required or is invalid:"
+	MSgErrorGitLeaksPermissionDenied            = "error: could not lock config file .git/config: Permission denied"
 	MsgErrorSeverityNotValid                    = "Type of severity not valid. See severities enable:"
 	MsgErrorAskForUserCancelled                 = "{HORUSEC_CLI} Operation was canceled by user"
 	MsgVulnerabilityTypeToShowInvalid           = "{HORUSEC_CLI} Error on validate vulnerability type is wrong type: "
@@ -77,4 +78,5 @@ const (
 	MsgErrorGetRelativePathFromFile          = "{HORUSEC_CLI} Error when get relative path of file"
 	MsgErrorGetDependencyCodeFilepathAndLine = "{HORUSEC_CLI} Error when get dependency code filepath and line"
 	MsgErrorGetDependencyInfo                = "{HORUSEC_CLI} Error when get dependency code info"
+	MsgErrorBundlerNotAccessDB               = "{HORUSEC_CLI} BundlerAudit cannot access database in github: "
 )

internal/services/formatters/csharp/dotnet_cli/formatter.go

@@ -150,16 +150,18 @@ func (f *Formatter) parseFieldByIndex(index int, fieldValue string, dependency *
 	}
 }
 
-// nolint: funlen // needs to be bigger
+// nolint:funlen // This method will mount the vulnerability struct and is necessary more lines
 func (f *Formatter) newVulnerability(dependency *dotnetDependency,
 	projectSubPath string) (*vulnerability.Vulnerability, error,
 ) {
-	code, absFilePath, line, err := f.getCodeFilePathAndLine(dependency, projectSubPath)
+	dependencyInfo, err := file.GetDependencyCodeFilepathAndLine(f.GetConfigProjectPath(), projectSubPath,
+		[]string{dependency.Name}, CsProjExt,
+	)
 	if err != nil {
 		return nil, err
 	}
 
-	relFilePath, err := filepath.Rel(f.GetConfigProjectPath(), absFilePath)
+	relFilePath, err := filepath.Rel(f.GetConfigProjectPath(), dependencyInfo.Path)
 	if err != nil {
 		return nil, err
 	}
@@ -170,29 +172,17 @@ func (f *Formatter) newVulnerability(dependency *dotnetDependency,
 		Confidence:   confidence.High,
 		RuleID:       vulnhash.HashRuleID(dependency.getDescription()),
 		Details:      dependency.getDescription(),
-		Code:         code,
-		File:         relFilePath,
-		Line:         line,
+		Code:         dependencyInfo.Code,
+		File:         f.removeHorusecFolder(relFilePath),
+		Line:         dependencyInfo.Line,
 		Severity:     dependency.getSeverity(),
 	}
 
-	vuln.DeprecatedHashes = f.getDeprecatedHashes(absFilePath, *vuln)
+	vuln.DeprecatedHashes = f.getDeprecatedHashes(dependencyInfo.Path, *vuln)
 
 	return f.SetCommitAuthor(vulnhash.Bind(vuln)), nil
 }
 
-func (f *Formatter) getCodeFilePathAndLine(dependency *dotnetDependency,
-	projectSubPath string) (string, string, string, error,
-) {
-	code, filePath, line, err := file.GetDependencyCodeFilepathAndLine(
-		f.GetConfigProjectPath(), projectSubPath, dependency.Name, CsProjExt,
-	)
-	if err != nil {
-		return "", "", "", err
-	}
-	return code, filePath, line, nil
-}
-
 func (f *Formatter) checkOutputErrors(output string, err error) (string, error) {
 	if err != nil {
 		return output, err