Skip to content

Commit

Permalink
cli: fix breaking change on vulnerability hashes
Browse files Browse the repository at this point in the history
On pr #636 we add the rule id on description of vulnerability, but the
Details of vulnerability is used to generate the vulnerability hash, so
adding the rule id on details generate a different hash which cause a
breaking change. So this commit remove the rule id prefix from Details
field of Vulnerability and also add a workaround to users that is
already using the new hash as a false positive and risk accept.
To support the two ways of hashing the vulnerability a new field was
added on Vulnerability struct that represents the breaking way, so we
generate the two hashes of vulnerability and when we set the
vulnerability to false positive/risk accept according to config file we
use the two hashes to match.

Fixes #680

Signed-off-by: Matheus Alcantara <matheus.alcantara@zup.com.br>
  • Loading branch information
matheusalcantarazup committed Oct 15, 2021
1 parent 28ea3ed commit 092e490
Show file tree
Hide file tree
Showing 5 changed files with 204 additions and 42 deletions.
34 changes: 21 additions & 13 deletions internal/controllers/analyzer/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/google/uuid"

"github.com/ZupIT/horusec-devkit/pkg/entities/analysis"
"github.com/ZupIT/horusec-devkit/pkg/entities/vulnerability"
enumsAnalysis "github.com/ZupIT/horusec-devkit/pkg/enums/analysis"
"github.com/ZupIT/horusec-devkit/pkg/enums/confidence"
"github.com/ZupIT/horusec-devkit/pkg/enums/languages"
Expand Down Expand Up @@ -446,8 +447,10 @@ func (a *Analyzer) logProjectSubPath(language languages.Language, subPath string
func (a *Analyzer) checkIfNoExistHashAndLog(list []string) {
for _, hash := range list {
existing := false
for keyAv := range a.analysis.AnalysisVulnerabilities {
if hash == a.analysis.AnalysisVulnerabilities[keyAv].Vulnerability.VulnHash {
for idx := range a.analysis.AnalysisVulnerabilities {
vulnHash := a.analysis.AnalysisVulnerabilities[idx].Vulnerability.VulnHash
vulnHashBg := a.analysis.AnalysisVulnerabilities[idx].Vulnerability.VulnHashInvalid
if hash == vulnHash || hash == vulnHashBg {
existing = true
break
}
Expand Down Expand Up @@ -484,21 +487,26 @@ func (a *Analyzer) getCustomOrDefaultImage(language languages.Language) string {
return fmt.Sprintf("%s/%s", images.DefaultRegistry, images.MapValues()[language])
}

func (a *Analyzer) SetFalsePositivesAndRiskAcceptInVulnerabilities(
listFalsePositive, listRiskAccept []string) *analysis.Analysis {
for key := range a.analysis.AnalysisVulnerabilities {
a.setVulnerabilityType(key, listFalsePositive, enumsVulnerability.FalsePositive)
a.setVulnerabilityType(key, listRiskAccept, enumsVulnerability.RiskAccepted)
// SetFalsePositivesAndRiskAcceptInVulnerabilities set analysis vulnerabiltieis to false
// positive or risk accept if the hash exists on falsePositive and riskAccept params.
func (a *Analyzer) SetFalsePositivesAndRiskAcceptInVulnerabilities(falsePositive, riskAccept []string) *analysis.Analysis {
for idx := range a.analysis.AnalysisVulnerabilities {
a.setVulnerabilityType(
&a.analysis.AnalysisVulnerabilities[idx].Vulnerability, falsePositive, enumsVulnerability.FalsePositive,
)
a.setVulnerabilityType(
&a.analysis.AnalysisVulnerabilities[idx].Vulnerability, riskAccept, enumsVulnerability.RiskAccepted,
)
}
return a.analysis
}

func (a *Analyzer) setVulnerabilityType(keyAnalysisVulnerabilities int,
listToCheck []string, vulnerabilityType enumsVulnerability.Type) {
currentHash := a.analysis.AnalysisVulnerabilities[keyAnalysisVulnerabilities].Vulnerability.VulnHash
for _, flagVulnerabilityHash := range listToCheck {
if flagVulnerabilityHash != "" && strings.TrimSpace(currentHash) == strings.TrimSpace(flagVulnerabilityHash) {
a.analysis.AnalysisVulnerabilities[keyAnalysisVulnerabilities].Vulnerability.Type = vulnerabilityType
func (a *Analyzer) setVulnerabilityType(vuln *vulnerability.Vulnerability, hashes []string, vulnType enumsVulnerability.Type) {
for _, hash := range hashes {
hash = strings.TrimSpace(hash)
// See vulnerability.Vulnerability.VulnHashInvalid docs for more info.
if hash != "" && (strings.TrimSpace(vuln.VulnHash) == hash || strings.TrimSpace(vuln.VulnHashInvalid) == hash) {
vuln.Type = vulnType
}
}
}
Expand Down
83 changes: 83 additions & 0 deletions internal/controllers/analyzer/analyzer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,20 @@ package analyzer
import (
"bytes"
"errors"
"fmt"
"io"
"os"
"path/filepath"
"testing"

"github.com/ZupIT/horusec-devkit/pkg/entities/analysis"
entitiesAnalysis "github.com/ZupIT/horusec-devkit/pkg/entities/analysis"
"github.com/ZupIT/horusec-devkit/pkg/entities/vulnerability"
vulnerabilityenum "github.com/ZupIT/horusec-devkit/pkg/enums/vulnerability"
"github.com/ZupIT/horusec-devkit/pkg/utils/logger"
horusecAPI "github.com/ZupIT/horusec/internal/services/horusec_api"
"github.com/ZupIT/horusec/internal/utils/mock"
vulnhash "github.com/ZupIT/horusec/internal/utils/vuln_hash"

"github.com/ZupIT/horusec/internal/entities/workdir"

Expand Down Expand Up @@ -65,6 +70,84 @@ func BenchmarkAnalyzerAnalyze(b *testing.B) {
}
}

func TestAnalyzerSetFalsePositivesAndRiskAcceptInVulnerabilities(t *testing.T) {
vuln := vulnerability.Vulnerability{
RuleID: "HS-TEST-1",
Line: "10",
Column: "20",
File: "testing",
Code: "testing",
Details: fmt.Sprintf("Test\nDescription testing"),
}
vulnhash.Bind(&vuln)

testcases := []struct {
name string
vulnerability vulnerability.Vulnerability
hashes []string
expectedType vulnerabilityenum.Type
}{
{
name: "ChangeCorrectHashToFalsePositive",
vulnerability: vuln,
hashes: []string{vuln.VulnHash},
expectedType: vulnerabilityenum.FalsePositive,
},
{
name: "ChangeBreakingHashToFalsePositive",
vulnerability: vuln,
hashes: []string{vuln.VulnHashInvalid},
expectedType: vulnerabilityenum.FalsePositive,
},
{
name: "ChangeCorrectHashToRiskAccept",
vulnerability: vuln,
hashes: []string{vuln.VulnHash},
expectedType: vulnerabilityenum.RiskAccepted,
},
{
name: "ChangeBreakingHashToRiskAccept",
vulnerability: vuln,
hashes: []string{vuln.VulnHashInvalid},
expectedType: vulnerabilityenum.RiskAccepted,
},
}

for _, tt := range testcases {
t.Run(tt.name, func(t *testing.T) {
analyzer := NewAnalyzer(config.New())

analyzer.analysis.AnalysisVulnerabilities = append(
analyzer.analysis.AnalysisVulnerabilities, analysis.AnalysisVulnerabilities{
AnalysisID: uuid.New(),
Vulnerability: tt.vulnerability,
},
)
var (
falsePositiveHashes []string
riskAcceptHashes []string
)

switch tt.expectedType {
case vulnerabilityenum.FalsePositive:
falsePositiveHashes = tt.hashes
case vulnerabilityenum.RiskAccepted:
riskAcceptHashes = tt.hashes
default:
t.Fatalf("invalid type %s", tt.expectedType)
}

analyzer.SetFalsePositivesAndRiskAcceptInVulnerabilities(falsePositiveHashes, riskAcceptHashes)

require.Len(t, analyzer.analysis.AnalysisVulnerabilities, len(tt.hashes))
for _, vuln := range analyzer.analysis.AnalysisVulnerabilities {
assert.Equal(t, tt.expectedType, vuln.Vulnerability.Type)
}

})
}
}

func TestNewAnalyzer(t *testing.T) {
t.Run("Should return type os struct correctly", func(t *testing.T) {
assert.IsType(t, &Analyzer{}, NewAnalyzer(&config.Config{}))
Expand Down
3 changes: 2 additions & 1 deletion internal/services/formatters/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,12 +237,13 @@ func (s *Service) AddNewVulnerabilityIntoAnalysis(vuln *vulnerability.Vulnerabil
func (s *Service) newVulnerabilityFromFinding(finding *engine.Finding, tool tools.Tool,
language languages.Language) *vulnerability.Vulnerability {
return &vulnerability.Vulnerability{
RuleID: finding.ID,
Line: strconv.Itoa(finding.SourceLocation.Line),
Column: strconv.Itoa(finding.SourceLocation.Column),
Confidence: confidence.Confidence(finding.Confidence),
File: s.removeHorusecFolder(finding.SourceLocation.Filename),
Code: s.GetCodeWithMaxCharacters(finding.CodeSample, finding.SourceLocation.Column),
Details: fmt.Sprintf("%s: %s\n%s", finding.ID, finding.Name, finding.Description),
Details: fmt.Sprintf("%s\n%s", finding.Name, finding.Description),
SecurityTool: tool,
Language: language,
Severity: severities.GetSeverityByString(finding.Severity),
Expand Down
Loading

0 comments on commit 092e490

Please sign in to comment.