feat(fortifyExecuteScan): further improvements to the SARIF generation (#3799)

* feat(fortfiyExecuteScan): proper XML unescaping, added rulepacks to SARIF, added kingdom/type/subtype to tags

* feat(fortifyExecuteScan): proper handling of severity, kinds, levels in SARIF

* fix(fortifyExecuteScan): edge case when handling properties taht could lead to a crash

* fix(fortifyExecuteScan): ensure SARIF processing is done after latest FPR is processed by SSC

cmd/fortifyExecuteScan.go

@@ -247,7 +247,15 @@ func runFortifyScan(config fortifyExecuteScanOptions, sys fortify.System, utils
 		return reports, fmt.Errorf(message+": %w", err)
 	}
 
-	//Place conversion beforehand, or audit will stop the pipeline and conversion will not take place?
+	log.Entry().Infof("Ensuring latest FPR is processed for project %v with version %v and project version ID %v", fortifyProjectName, fortifyProjectVersion, projectVersion.ID)
+	// Ensure latest FPR is processed
+	err = verifyScanResultsFinishedUploading(config, sys, projectVersion.ID, buildLabel, filterSet,
+		10*time.Second, time.Duration(config.PollingMinutes)*time.Minute)
+	if err != nil {
+		return reports, err
+	}
+
+	// SARIF conversion done after latest FPR is processed, but before the compliance is checked
 	if config.ConvertToSarif {
 		resultFilePath := fmt.Sprintf("%vtarget/result.fpr", config.ModulePath)
 		log.Entry().Info("Calling conversion to SARIF function.")
@@ -262,13 +270,8 @@ func runFortifyScan(config fortifyExecuteScanOptions, sys fortify.System, utils
 		}
 		reports = append(reports, paths...)
 	}
+
 	log.Entry().Infof("Starting audit status check on project %v with version %v and project version ID %v", fortifyProjectName, fortifyProjectVersion, projectVersion.ID)
-	// Ensure latest FPR is processed
-	err = verifyScanResultsFinishedUploading(config, sys, projectVersion.ID, buildLabel, filterSet,
-		10*time.Second, time.Duration(config.PollingMinutes)*time.Minute)
-	if err != nil {
-		return reports, err
-	}
 	err, paths := verifyFFProjectCompliance(config, utils, sys, project, projectVersion, filterSet, influx, auditStatus)
 	reports = append(reports, paths...)
 	return reports, err

pkg/format/sarif.go

@@ -24,6 +24,7 @@ type Runs struct {
 type Results struct {
 	RuleID              string              `json:"ruleId"`
 	RuleIndex           int                 `json:"ruleIndex"`
+	Kind                string              `json:"kind,omitempty"`
 	Level               string              `json:"level,omitempty"`
 	Message             *Message            `json:"message,omitempty"`
 	AnalysisTarget      *ArtifactLocation   `json:"analysisTarget,omitempty"`
@@ -102,14 +103,16 @@ type SarifProperties struct {
 // Tool these structs are relevant to the Tool object
 type Tool struct {
 	Driver     Driver   `json:"driver"`
+	Extensions []Driver `json:"extensions"`
 }
 
 // Driver meta information for the scan and tool context
 type Driver struct {
 	Name                string                `json:"name"`
 	Version             string                `json:"version"`
+	GUID                string                `json:"guid,omitempty"`
 	InformationUri      string                `json:"informationUri,omitempty"`
-	Rules               []SarifRule           `json:"rules"`
+	Rules               []SarifRule           `json:"rules,omitempty"`
 	SupportedTaxonomies []SupportedTaxonomies `json:"supportedTaxonomies,omitempty"`
 }
 
@@ -191,6 +194,8 @@ type SupportedTaxonomies struct {
 type DefaultConfiguration struct {
 	Properties DefaultProperties `json:"properties,omitempty"`
 	Level      string            `json:"level,omitempty"` //This exists in the template, but not sure how it is populated. TODO.
+	Enabled    bool              `json:"enabled,omitempty"`
+	Rank       float64           `json:"rank,omitempty"`
 }
 
 // DefaultProperties
@@ -223,6 +228,7 @@ type SarifRuleProperties struct {
 	Probability      string   `json:"probability,omitempty"`
 	Tags             []string `json:"tags,omitempty"`
 	Precision        string   `json:"precision,omitempty"`
+	SecuritySeverity string   `json:"security-severity,omitempty"` //used by GHAS to defined the tag (low,medium,high)
 }
 
 // Invocations These structs are relevant to the Invocations object

pkg/fortify/fpr_to_sarif.go

@@ -8,6 +8,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strconv"
 	"strings"
 
 	"github.com/piper-validation/fortify-client-go/models"
@@ -112,14 +113,14 @@ type ClassInfo struct {
 	Type            string   `xml:"Type"`
 	Subtype         string   `xml:"Subtype,omitempty"`
 	AnalyzerName    string   `xml:"AnalyzerName"`
-	DefaultSeverity string   `xml:"DefaultSeverity"`
+	DefaultSeverity float64  `xml:"DefaultSeverity"`
 }
 
 // InstanceInfo
 type InstanceInfo struct {
 	XMLName          xml.Name `xml:"InstanceInfo"`
 	InstanceID       string   `xml:"InstanceID"`
-	InstanceSeverity string   `xml:"InstanceSeverity"`
+	InstanceSeverity float64  `xml:"InstanceSeverity"`
 	Confidence       string   `xml:"Confidence"`
 }
 
@@ -580,8 +581,18 @@ func Parse(sys System, project *models.Project, projectVersion *models.ProjectVe
 			idArray = append(idArray, fvdl.Vulnerabilities.Vulnerability[i].ClassInfo.Subtype)
 		}
 		result.RuleID = "fortify-" + strings.Join(idArray, "/")
-		// end handle result
+		result.Kind = "fail" // Default value, Level must not be set if kind is not fail
-		result.Level = "none" //TODO
+		// This is an "easy" treatment of result.Level. It does not follow the spec exactly, but the idea is there
+		// An exact processing algorithm can be found here https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317648
+		if fvdl.Vulnerabilities.Vulnerability[i].InstanceInfo.InstanceSeverity >= 3.0 {
+			result.Level = "error"
+		} else if fvdl.Vulnerabilities.Vulnerability[i].InstanceInfo.InstanceSeverity >= 1.5 {
+			result.Level = "warning"
+		} else if fvdl.Vulnerabilities.Vulnerability[i].InstanceInfo.InstanceSeverity < 1.5 {
+			result.Level = "note"
+		} else {
+			result.Level = "none"
+		}
 		//get message
 		for j := 0; j < len(fvdl.Description); j++ {
 			if fvdl.Description[j].ClassID == fvdl.Vulnerabilities.Vulnerability[i].ClassInfo.ClassID {
@@ -682,7 +693,7 @@ func Parse(sys System, project *models.Project, projectVersion *models.ProjectVe
 					location = *tfloc
 					//set Kinds
 					threadFlowLocation.Location = tfloc
-					threadFlowLocation.Kinds = append(threadFlowLocation.Kinds, "review") //TODO
+					//threadFlowLocation.Kinds = append(threadFlowLocation.Kinds, "review") //TODO
 					threadFlowLocation.Index = 0 // to be safe?
 					tfla = append(tfla, threadFlowLocation)
 
@@ -770,7 +781,7 @@ func Parse(sys System, project *models.Project, projectVersion *models.ProjectVe
 
 		//handle properties
 		prop := new(format.SarifProperties)
-		prop.InstanceSeverity = fvdl.Vulnerabilities.Vulnerability[i].InstanceInfo.InstanceSeverity
+		prop.InstanceSeverity = strconv.FormatFloat(fvdl.Vulnerabilities.Vulnerability[i].InstanceInfo.InstanceSeverity, 'f', 1, 64)
 		prop.Confidence = fvdl.Vulnerabilities.Vulnerability[i].InstanceInfo.Confidence
 		prop.InstanceID = fvdl.Vulnerabilities.Vulnerability[i].InstanceInfo.InstanceID
 		//Get the audit data
@@ -836,12 +847,15 @@ func Parse(sys System, project *models.Project, projectVersion *models.ProjectVe
 				sarifRule.Name = strings.Join(nameArray, "")
 				defaultConfig := new(format.DefaultConfiguration)
 				defaultConfig.Level = "warning" // Default value
-				defaultConfig.Properties.DefaultSeverity = fvdl.Vulnerabilities.Vulnerability[j].ClassInfo.DefaultSeverity
+				defaultConfig.Enabled = true    // Default value
+				defaultConfig.Rank = -1.0       // Default value
+				defaultConfig.Properties.DefaultSeverity = strconv.FormatFloat(fvdl.Vulnerabilities.Vulnerability[j].ClassInfo.DefaultSeverity, 'f', 1, 64)
 				sarifRule.DefaultConfiguration = defaultConfig
 
 				//Descriptions
 				for j := 0; j < len(fvdl.Description); j++ {
 					if fvdl.Description[j].ClassID == sarifRule.GUID {
+						//rawAbstract := strings.Join(idArray, "/")
 						rawAbstract := unescapeXML(fvdl.Description[j].Abstract.Text)
 						rawExplanation := unescapeXML(fvdl.Description[j].Explanation.Text)
 
@@ -858,7 +872,7 @@ func Parse(sys System, project *models.Project, projectVersion *models.ProjectVe
 								}
 								// If Description has a CustomDescription, add it for good measure
 								if fvdl.Description[j].CustomDescription.RuleID != "" {
-									rawExplanation = rawExplanation + "\n;" + fvdl.Description[j].CustomDescription.Explanation.Text
+									rawExplanation = rawExplanation + " \n; " + unescapeXML(fvdl.Description[j].CustomDescription.Explanation.Text)
 								}
 								sd := new(format.Message)
 								sd.Text = rawAbstract
@@ -900,8 +914,8 @@ func Parse(sys System, project *models.Project, projectVersion *models.ProjectVe
 					}
 				}
 				var ruleProp *format.SarifRuleProperties
-				if len(propArray) != 0 {
 				ruleProp = new(format.SarifRuleProperties)
+				if len(propArray) != 0 {
 					for j := 0; j < len(propArray); j++ {
 						if propArray[j][0] == "Accuracy" {
 							ruleProp.Accuracy = propArray[j][1]
@@ -912,11 +926,29 @@ func Parse(sys System, project *models.Project, projectVersion *models.ProjectVe
 						}
 					}
 				}
+
+				// Add each part of the "name" in the tags
+				if fvdl.Vulnerabilities.Vulnerability[j].ClassInfo.Kingdom != "" {
+					ruleProp.Tags = append(ruleProp.Tags, fvdl.Vulnerabilities.Vulnerability[j].ClassInfo.Kingdom)
+				}
+				if fvdl.Vulnerabilities.Vulnerability[j].ClassInfo.Type != "" {
+					ruleProp.Tags = append(ruleProp.Tags, fvdl.Vulnerabilities.Vulnerability[j].ClassInfo.Type)
+				}
+				if fvdl.Vulnerabilities.Vulnerability[j].ClassInfo.Subtype != "" {
+					ruleProp.Tags = append(ruleProp.Tags, fvdl.Vulnerabilities.Vulnerability[j].ClassInfo.Subtype)
+				}
+
+				//Add the SecuritySeverity parameter for GHAS tagging
+				ruleProp.SecuritySeverity = strconv.FormatFloat(2*fvdl.Vulnerabilities.Vulnerability[j].InstanceInfo.InstanceSeverity, 'f', 1, 64)
+
 				sarifRule.Properties = ruleProp
 
 				//relationships: will most likely require some expansion
 				//One relationship per CWE id
 				for j := 0; j < len(cweIds); j++ {
+					if cweIds[j] == "None" {
+						continue
+					}
 					sarifRule.Properties.Tags = append(sarifRule.Properties.Tags, "external/cwe/cwe-"+cweIds[j])
 
 					rls := *new(format.Relationships)
@@ -945,6 +977,15 @@ func Parse(sys System, project *models.Project, projectVersion *models.ProjectVe
 	sTax.Guid = "25F72D7E-8A92-459D-AD67-64853F788765"
 	tool.Driver.SupportedTaxonomies = append(tool.Driver.SupportedTaxonomies, sTax)
 
+	//Add additional rulepacks
+	for pack := 0; pack < len(fvdl.EngineData.RulePacks); pack++ {
+		extension := *new(format.Driver)
+		extension.Name = fvdl.EngineData.RulePacks[pack].Name
+		extension.Version = fvdl.EngineData.RulePacks[pack].Version
+		extension.GUID = fvdl.EngineData.RulePacks[pack].RulePackID
+		tool.Extensions = append(tool.Extensions, extension)
+	}
+
 	//Finalize: tool
 	sarif.Runs[0].Tool = tool
 
@@ -1117,6 +1158,14 @@ func Parse(sys System, project *models.Project, projectVersion *models.ProjectVe
 }
 
 func integrateAuditData(ruleProp *format.SarifProperties, issueInstanceID string, sys System, project *models.Project, projectVersion *models.ProjectVersion, auditData []*models.ProjectVersionIssue, filterSet *models.FilterSet, oneRequestPerIssue bool) error {
+	// Set default values
+	ruleProp.Audited = false
+	ruleProp.FortifyCategory = "Unknown"
+	ruleProp.ToolSeverity = "Unknown"
+	ruleProp.ToolState = "Unreviewed"
+	ruleProp.ToolSeverityIndex = 0
+	ruleProp.ToolStateIndex = 0
+	// These default values allow for the property bag to be filled even if an error happens later. They all should be overwritten by a normal course of the progrma.
 	if sys == nil {
 		err := errors.New("no system instance, lookup impossible for " + issueInstanceID)
 		return err
@@ -1144,6 +1193,17 @@ func integrateAuditData(ruleProp *format.SarifProperties, issueInstanceID string
 	if len(data) != 1 { //issueInstanceID is supposedly unique so len(data) = 1
 		return errors.New("not exactly 1 issue found, found " + fmt.Sprint(len(data)))
 	}
+	if filterSet != nil {
+		for i := 0; i < len(filterSet.Folders); i++ {
+			if filterSet.Folders[i].GUID == *data[0].FolderGUID {
+				ruleProp.FortifyCategory = filterSet.Folders[i].Name
+				break
+			}
+		}
+	} else {
+		err := errors.New("no filter set defined, category will be missing from " + issueInstanceID)
+		return err
+	}
 	ruleProp.Audited = data[0].Audited
 	ruleProp.ToolSeverity = *data[0].Friority
 	switch ruleProp.ToolSeverity {
@@ -1184,17 +1244,6 @@ func integrateAuditData(ruleProp *format.SarifProperties, issueInstanceID string
 		}
 		ruleProp.ToolAuditMessage = unescapeXML(*commentData[0].Comment)
 	}
-	if filterSet != nil {
-		for i := 0; i < len(filterSet.Folders); i++ {
-			if filterSet.Folders[i].GUID == *data[0].FolderGUID {
-				ruleProp.FortifyCategory = filterSet.Folders[i].Name
-				break
-			}
-		}
-	} else {
-		err := errors.New("no filter set defined, category will be missing from " + issueInstanceID)
-		return err
-	}
 	return nil
 }
 
@@ -1233,9 +1282,9 @@ func handleSnippet(snippetType string, snippet string) string {
 func unescapeXML(input string) string {
 	raw := input
 	// Post-treat string to change the XML escaping generated by Unmarshal
+	raw = strings.ReplaceAll(raw, "&amp;", "&")
 	raw = strings.ReplaceAll(raw, "&lt;", "<")
 	raw = strings.ReplaceAll(raw, "&gt;", ">")
-	raw = strings.ReplaceAll(raw, "&amp;", "&")
 	raw = strings.ReplaceAll(raw, "&apos;", "'")
 	raw = strings.ReplaceAll(raw, "&quot;", "\"")
 	return raw