Skip to content

Commit

Permalink
Fix bugs when converting results to SARIF
Browse files Browse the repository at this point in the history
  • Loading branch information
attiasas committed Nov 14, 2024
1 parent 49fd385 commit 41bd0a9
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 31 deletions.
77 changes: 72 additions & 5 deletions utils/formats/sarifutils/sarifutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,17 @@ func GetToolVersion(run *sarif.Run) string {
return ""
}

func CopyRun(run *sarif.Run) *sarif.Run {
copy := CopyRunMetadata(run)
if copy.Tool.Driver != nil {
copy.Tool.Driver.Rules = CopyRules(run.Tool.Driver.Rules...)
}
for _, result := range run.Results {
copy.Results = append(copy.Results, CopyResult(result))
}
return copy
}

func CopyRunMetadata(run *sarif.Run) (copied *sarif.Run) {
if run == nil {
return
Expand Down Expand Up @@ -64,6 +75,21 @@ func CopyRunMetadata(run *sarif.Run) (copied *sarif.Run) {
return
}

func CopyRules(rules ...*sarif.ReportingDescriptor) (copied []*sarif.ReportingDescriptor) {
for _, rule := range rules {
cloned := sarif.NewRule(rule.ID)
cloned.HelpURI = copyStrAttribute(rule.HelpURI)
cloned.Name = copyStrAttribute(rule.Name)
cloned.ShortDescription = copyMultiMsgAttribute(rule.ShortDescription)
cloned.FullDescription = copyMultiMsgAttribute(rule.FullDescription)
cloned.Help = copyMultiMsgAttribute(rule.Help)
cloned.Properties = rule.Properties
cloned.MessageStrings = rule.MessageStrings
copied = append(copied, cloned)
}
return
}

func GetRunToolFullName(run *sarif.Run) string {
if run.Tool.Driver != nil && run.Tool.Driver.FullName != nil {
return *run.Tool.Driver.FullName
Expand Down Expand Up @@ -134,9 +160,9 @@ func CopyResult(result *sarif.Result) *sarif.Result {
RuleIndex: result.RuleIndex,
Kind: result.Kind,
Fingerprints: result.Fingerprints,
CodeFlows: result.CodeFlows,
CodeFlows: copyCodeFlows(result.CodeFlows...),
Level: result.Level,
Message: result.Message,
Message: copyMsgAttribute(result.Message),
PropertyBag: result.PropertyBag,
}
for _, location := range result.Locations {
Expand All @@ -145,6 +171,47 @@ func CopyResult(result *sarif.Result) *sarif.Result {
return copied
}

func copyCodeFlows(flows ...*sarif.CodeFlow) []*sarif.CodeFlow {
var copied []*sarif.CodeFlow
for _, flow := range flows {
copied = append(copied, copyCodeFlow(flow))
}
return copied
}

func copyCodeFlow(flow *sarif.CodeFlow) *sarif.CodeFlow {
copied := &sarif.CodeFlow{}
for _, threadFlow := range flow.ThreadFlows {
copied.ThreadFlows = append(copied.ThreadFlows, copyThreadFlow(threadFlow))
}
return copied
}

func copyThreadFlow(threadFlow *sarif.ThreadFlow) *sarif.ThreadFlow {
copied := &sarif.ThreadFlow{}
for _, location := range threadFlow.Locations {
copied.Locations = append(copied.Locations, sarif.NewThreadFlowLocation().WithLocation(CopyLocation(location.Location)))
}
return copied
}

func copyMsgAttribute(attr sarif.Message) sarif.Message {
return sarif.Message{
Text: copyStrAttribute(attr.Text),
Markdown: copyStrAttribute(attr.Markdown),
}
}

func copyMultiMsgAttribute(attr *sarif.MultiformatMessageString) *sarif.MultiformatMessageString {
if attr == nil {
return nil
}
return &sarif.MultiformatMessageString{
Text: copyStrAttribute(attr.Text),
Markdown: copyStrAttribute(attr.Markdown),
}
}

func copyStrAttribute(attr *string) *string {
if attr == nil {
return nil
Expand Down Expand Up @@ -190,9 +257,9 @@ func CopyLocation(location *sarif.Location) *sarif.Location {
copied.Properties = location.Properties
for _, logicalLocation := range location.LogicalLocations {
copied.LogicalLocations = append(copied.LogicalLocations, &sarif.LogicalLocation{
Name: logicalLocation.Name,
FullyQualifiedName: logicalLocation.FullyQualifiedName,
DecoratedName: logicalLocation.DecoratedName,
Name: copyStrAttribute(logicalLocation.Name),
FullyQualifiedName: copyStrAttribute(logicalLocation.FullyQualifiedName),
DecoratedName: copyStrAttribute(logicalLocation.DecoratedName),
Kind: logicalLocation.Kind,
PropertyBag: logicalLocation.PropertyBag,
})
Expand Down
58 changes: 34 additions & 24 deletions utils/results/conversion/sarifparser/sarifparser.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,13 @@ func (sc *CmdResultsSarifConverter) ParseNewTargetResults(target results.ScanTar
func (sc *CmdResultsSarifConverter) createScaRun(target results.ScanTarget, errorCount int) *sarif.Run {
run := sarif.NewRunWithInformationURI(ScaScannerToolName, utils.BaseDocumentationURL+"sca")
run.Tool.Driver.Version = &sc.xrayVersion
wd := target.Target
if sc.currentCmdType.IsTargetBinary() {
// For binary, the target is a file and not a directory
wd = filepath.Dir(wd)
}
run.Invocations = append(run.Invocations, sarif.NewInvocation().
WithWorkingDirectory(sarif.NewSimpleArtifactLocation(target.Target)).
WithWorkingDirectory(sarif.NewSimpleArtifactLocation(wd)).
WithExecutionSuccess(errorCount == 0),
)
return run
Expand Down Expand Up @@ -240,7 +245,7 @@ func addSarifScaVulnerability(cmdType utils.CommandType, sarifResults *[]*sarif.
if err != nil {
return err
}
currentResults, currentRule := parseScaToSarifFormat(cmdType, vulnerability.IssueId, vulnerability.Summary, markdownDescription, maxCveScore, getScaIssueSarifHeadline, cves, severity, applicabilityStatus, impactedPackagesName, impactedPackagesVersion, fixedVersions, directComponents)
currentResults, currentRule := parseScaToSarifFormat(cmdType, vulnerability.IssueId, vulnerability.Summary, markdownDescription, maxCveScore, getScaVulnerabilitySarifHeadline, cves, severity, applicabilityStatus, impactedPackagesName, impactedPackagesVersion, fixedVersions, directComponents)
cveImpactedComponentRuleId := results.GetScaIssueId(impactedPackagesName, impactedPackagesVersion, results.GetIssueIdentifier(cves, vulnerability.IssueId, "_"))
if _, ok := (*rules)[cveImpactedComponentRuleId]; !ok {
// New Rule
Expand All @@ -261,7 +266,7 @@ func addSarifScaSecurityViolation(cmdType utils.CommandType, sarifResults *[]*sa
if err != nil {
return err
}
currentResults, currentRule := parseScaToSarifFormat(cmdType, violation.IssueId, violation.Summary, markdownDescription, maxCveScore, getScaIssueSarifHeadline, cves, severity, applicabilityStatus, impactedPackagesName, impactedPackagesVersion, fixedVersions, directComponents, violation.WatchName)
currentResults, currentRule := parseScaToSarifFormat(cmdType, violation.IssueId, violation.Summary, markdownDescription, maxCveScore, getScaSecurityViolationSarifHeadline, cves, severity, applicabilityStatus, impactedPackagesName, impactedPackagesVersion, fixedVersions, directComponents, violation.WatchName)
cveImpactedComponentRuleId := results.GetScaIssueId(impactedPackagesName, impactedPackagesVersion, results.GetIssueIdentifier(cves, violation.IssueId, "_"))
if _, ok := (*rules)[cveImpactedComponentRuleId]; !ok {
// New Rule
Expand Down Expand Up @@ -396,10 +401,14 @@ func getDirectDependenciesFormatted(directDependencies []formats.ComponentRow) (
return strings.TrimSuffix(formattedDirectDependencies.String(), "<br/>"), nil
}

func getScaIssueSarifHeadline(depName, version, issueId string) string {
func getScaVulnerabilitySarifHeadline(depName, version, issueId string) string {
return fmt.Sprintf("[%s] %s %s", issueId, depName, version)
}

func getScaSecurityViolationSarifHeadline(depName, version, key string) string {
return fmt.Sprintf("Security violation %s", getScaVulnerabilitySarifHeadline(depName, version, key))
}

func getXrayLicenseSarifHeadline(depName, version, key string) string {
return fmt.Sprintf("License violation [%s] in %s %s", key, depName, version)
}
Expand All @@ -417,21 +426,21 @@ func getScaLicenseViolationMarkdown(depName, version, key string, directDependen
}

func patchRunsToPassIngestionRules(cmdType utils.CommandType, subScanType utils.SubScanType, patchBinaryPaths bool, target results.ScanTarget, runs ...*sarif.Run) []*sarif.Run {
// Since we run in temp directories files should be relative
// Patch by converting the file paths to relative paths according to the invocations
convertPaths(cmdType, subScanType, runs...)
patchedRuns := []*sarif.Run{}
// Patch changes may alter the original run, so we will create a new run for each
for _, run := range runs {
patched := sarifutils.CopyRunMetadata(run)
patched := sarifutils.CopyRun(run)
// Since we run in temp directories files should be relative
// Patch by converting the file paths to relative paths according to the invocations
convertPaths(cmdType, subScanType, patched)
if cmdType.IsTargetBinary() && subScanType == utils.SecretsScan {
// Patch the tool name in case of binary scan
sarifutils.SetRunToolName(BinarySecretScannerToolName, patched)
}
if patched.Tool.Driver != nil {
patched.Tool.Driver.Rules = patchRules(cmdType, subScanType, run.Tool.Driver.Rules...)
patched.Tool.Driver.Rules = patchRules(cmdType, subScanType, patched.Tool.Driver.Rules...)
}
patched.Results = patchResults(cmdType, subScanType, patchBinaryPaths, target, run, run.Results...)
patched.Results = patchResults(cmdType, subScanType, patchBinaryPaths, target, patched, patched.Results...)
patchedRuns = append(patchedRuns, patched)
}
return patchedRuns
Expand Down Expand Up @@ -470,28 +479,20 @@ func patchDockerSecretLocations(result *sarif.Result) {
func patchRules(commandType utils.CommandType, subScanType utils.SubScanType, rules ...*sarif.ReportingDescriptor) (patched []*sarif.ReportingDescriptor) {
patched = []*sarif.ReportingDescriptor{}
for _, rule := range rules {
cloned := sarif.NewRule(rule.ID)
if rule.Name != nil && rule.ID == *rule.Name {
// SARIF1001 - if both 'id' and 'name' are present, they must be different. If they are identical, the tool must omit the 'name' property.
cloned.Name = rule.Name
rule.Name = nil
}
cloned.ShortDescription = rule.ShortDescription
if commandType.IsTargetBinary() && subScanType == utils.SecretsScan {
// Patch the rule name in case of binary scan
sarifutils.SetRuleShortDescriptionText(fmt.Sprintf("[Secret in Binary found] %s", sarifutils.GetRuleShortDescriptionText(rule)), cloned)
sarifutils.SetRuleShortDescriptionText(fmt.Sprintf("[Secret in Binary found] %s", sarifutils.GetRuleShortDescriptionText(rule)), rule)
}
cloned.FullDescription = rule.FullDescription
cloned.Help = rule.Help
if cloned.Help == nil {
if rule.Help == nil {
// Github code scanning ingestion rules rejects rules without help content.
// Patch by transferring the full description to the help field.
cloned.Help = rule.FullDescription
rule.Help = rule.FullDescription
}
cloned.HelpURI = rule.HelpURI
cloned.Properties = rule.Properties
cloned.MessageStrings = rule.MessageStrings

patched = append(patched, cloned)
patched = append(patched, rule)
}
return
}
Expand Down Expand Up @@ -734,7 +735,7 @@ func calculateResultFingerprints(resultType utils.CommandType, run *sarif.Run, r
if !resultType.IsTargetBinary() {
return nil
}
ids := []string{sarifutils.GetRunToolName(run), sarifutils.GetResultRuleId(result)}
ids := []string{sarifutils.GetRunToolName(run), sarifutils.GetResultRuleId(result), getResultWatches(result)}
for _, location := range sarifutils.GetResultFileLocations(result) {
ids = append(ids, strings.ReplaceAll(location, string(filepath.Separator), "/"))
}
Expand All @@ -747,3 +748,12 @@ func calculateResultFingerprints(resultType utils.CommandType, run *sarif.Run, r
sarifutils.SetResultFingerprint(jfrogFingerprintAlgorithmName, hashValue, result)
return nil
}

func getResultWatches(result *sarif.Result) (watches string) {
if watchesProperty, ok := result.Properties[WatchSarifPropertyKey]; ok {
if watchesValue, ok := watchesProperty.(string); ok {
return watchesValue
}
}
return
}
9 changes: 7 additions & 2 deletions utils/results/conversion/sarifparser/sarifparser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,13 @@ func TestGetComponentSarifLocation(t *testing.T) {
}

func TestGetVulnerabilityOrViolationSarifHeadline(t *testing.T) {
assert.Equal(t, "[CVE-2022-1234] loadsh 1.4.1", getScaIssueSarifHeadline("loadsh", "1.4.1", "CVE-2022-1234"))
assert.NotEqual(t, "[CVE-2022-1234] comp 1.4.1", getScaIssueSarifHeadline("comp", "1.2.1", "CVE-2022-1234"))
assert.Equal(t, "[CVE-2022-1234] loadsh 1.4.1", getScaVulnerabilitySarifHeadline("loadsh", "1.4.1", "CVE-2022-1234"))
assert.NotEqual(t, "[CVE-2022-1234] comp 1.4.1", getScaVulnerabilitySarifHeadline("comp", "1.2.1", "CVE-2022-1234"))
}

func TestGetScaSecurityViolationSarifHeadline(t *testing.T) {
assert.Equal(t, "Security violation [CVE-2022-1234] loadsh 1.4.1", getScaSecurityViolationSarifHeadline("loadsh", "1.4.1", "CVE-2022-1234"))
assert.NotEqual(t, "Security violation [CVE-2022-1234] comp 1.2.1", getScaSecurityViolationSarifHeadline("comp", "1.2.1", "CVE-2022-1234"))
}

func TestGetXrayLicenseSarifHeadline(t *testing.T) {
Expand Down

0 comments on commit 41bd0a9

Please sign in to comment.