Fix sonar report invalid text range (AST-170299)

internal/commands/result.go

@@ -1,6 +1,7 @@
 package commands
 
 import (
+	"bufio"
 	"encoding/json"
 	"fmt"
 	"html"
@@ -14,6 +15,7 @@ import (
 	"strings"
 	"text/template"
 	"time"
+	"unicode/utf8"
 
 	"github.com/MakeNowJust/heredoc"
 	"github.com/checkmarx/ast-cli/internal/commands/util"
@@ -132,6 +134,18 @@ const (
 	statusCompleted           = "Completed"
 	statusPartial             = "Partial"
 	statusFailed              = "Failed"
+	maxSonarLineBytes         = 1024 * 1024
+	byteOrderMarkRune         = rune(0xFEFF)
+	parentDir                 = ".."
+)
+
+// lineStatus reports how much of a location the source file could confirm.
+type lineStatus int
+
+const (
+	lineStatusFileUnknown lineStatus = iota
+	lineStatusLineMissing
+	lineStatusOK
 )
 
 var (
@@ -2332,6 +2346,8 @@ func parseSonar(results *wrappers.ScanResultsCollection) ([]wrappers.SonarIssues
 	var sonarIssues []wrappers.SonarIssues
 	var sonarRules []wrappers.SonarRules
 	seenRuleIDs := make(map[string]bool) // Track already added rule IDs
+	// Shared across the whole export so each source file is read at most once.
+	lineIndex := newSonarLineIndex()
 
 	if results != nil {
 		for _, result := range results.Results {
@@ -2346,8 +2362,15 @@ func parseSonar(results *wrappers.ScanResultsCollection) ([]wrappers.SonarIssues
 			engineType := strings.TrimSpace(result.Type)
 
 			if engineType == commonParams.SastType {
-				auxIssue.PrimaryLocation = parseSonarPrimaryLocation(result)
-				auxIssue.SecondaryLocations = parseSonarSecondaryLocations(result)
+				auxIssue.PrimaryLocation = parseSonarPrimaryLocation(result, lineIndex)
+				auxIssue.SecondaryLocations = parseSonarSecondaryLocations(result, lineIndex)
+				if auxIssue.PrimaryLocation.FilePath == "" {
+					// filePath is mandatory on the primary location, so an issue
+					// without one cannot be imported at all. Skipping it keeps
+					// the rest of the report loadable instead of having
+					// SonarQube reject every issue in the file.
+					continue
+				}
 				sonarIssues = append(sonarIssues, auxIssue)
 			} else if engineType == commonParams.KicsType {
 				auxIssue.PrimaryLocation = parseLocationKics(result)
@@ -2376,7 +2399,7 @@ func parseContainersSonar(result *wrappers.ScanResult) wrappers.SonarLocation {
 	textRange.EndColumn = 2
 	textRange.StartLine = 1
 	textRange.EndLine = 2
-	auxLocation.TextRange = textRange
+	auxLocation.TextRange = &textRange
 	return auxLocation
 }
 
@@ -2388,7 +2411,7 @@ func parseSscsSonar(result *wrappers.ScanResult, sonarIssue *wrappers.SonarIssue
 	textRange.StartColumn = 1
 	textRange.EndColumn = 2
 	textRange.StartLine = result.ScanResultData.Line
-	sonarIssue.PrimaryLocation.TextRange = textRange
+	sonarIssue.PrimaryLocation.TextRange = &textRange
 	return *sonarIssue
 }
 
@@ -2472,7 +2495,7 @@ func parseScaSonarLocations(result *wrappers.ScanResult) []wrappers.SonarIssues
 		textRange.StartLine = 1
 		textRange.EndLine = 2
 
-		primaryLocation.TextRange = textRange
+		primaryLocation.TextRange = &textRange
 
 		issueByLocation.PrimaryLocation = primaryLocation
 
@@ -2490,48 +2513,185 @@ func parseLocationKics(results *wrappers.ScanResult) wrappers.SonarLocation {
 	auxTextRange.StartLine = results.ScanResultData.Line
 	auxTextRange.StartColumn = 0
 	auxTextRange.EndColumn = 1
-	auxLocation.TextRange = auxTextRange
+	auxLocation.TextRange = &auxTextRange
 	return auxLocation
 }
 
-func parseSonarPrimaryLocation(results *wrappers.ScanResult) wrappers.SonarLocation {
+// sonarLineIndex caches source line lengths so each report file is read at most once per export.
+type sonarLineIndex struct {
+	baseDir string
+	files   map[string][]uint
+}
+
+func newSonarLineIndex() *sonarLineIndex {
+	baseDir, err := os.Getwd()
+	if err == nil {
+		// Resolved once here so resolveSourcePath can compare against it directly on every call.
+		baseDir, err = filepath.EvalSymlinks(baseDir)
+	}
+	if err != nil {
+		baseDir = ""
+	}
+	return &sonarLineIndex{baseDir: baseDir, files: make(map[string][]uint)}
+}
+
+// resolveLine reports whether the given 1-based line of fileName exists and, when it does, how many characters it holds.
+func (index *sonarLineIndex) resolveLine(fileName string, line uint) (length uint, status lineStatus) {
+	if fileName == "" {
+		return 0, lineStatusFileUnknown
+	}
+
+	lengths, cached := index.files[fileName]
+	if !cached {
+		lengths = index.readLineLengths(fileName)
+		index.files[fileName] = lengths
+	}
+	if lengths == nil {
+		return 0, lineStatusFileUnknown
+	}
+
+	if line == 0 || line > uint(len(lengths)) {
+		return 0, lineStatusLineMissing
+	}
+	return lengths[line-1], lineStatusOK
+}
+
+// resolveSourcePath cleans a report file path and confines it to baseDir, rejecting anything that escapes.
+func (index *sonarLineIndex) resolveSourcePath(fileName string) (path string, ok bool) {
+	if index.baseDir == "" {
+		return "", false
+	}
+
+	relative := filepath.FromSlash(strings.TrimLeft(fileName, "/\\"))
+	if relative == "" || filepath.IsAbs(relative) || filepath.VolumeName(relative) != "" {
+		return "", false
+	}
+
+	cleaned := filepath.Join(index.baseDir, relative)
+
+	// EvalSymlinks confines containment to the real path, not a lexical one.
+	realPath, err := filepath.EvalSymlinks(cleaned)
+	if err != nil {
+		return "", false
+	}
+
+	inside, err := filepath.Rel(index.baseDir, realPath)
+	if err != nil || inside == parentDir || strings.HasPrefix(inside, parentDir+string(os.PathSeparator)) {
+		return "", false
+	}
+	return realPath, true
+}
+
+// readLineLengths returns the character length of each line of a file, or nil when it cannot be read in full.
+func (index *sonarLineIndex) readLineLengths(fileName string) []uint {
+	path, ok := index.resolveSourcePath(fileName)
+	if !ok {
+		return nil
+	}
+
+	file, err := os.Open(path)
+	if err != nil {
+		return nil
+	}
+	defer func() { _ = file.Close() }()
+
+	scanner := bufio.NewScanner(file)
+	scanner.Buffer(make([]byte, 0, bufio.MaxScanTokenSize), maxSonarLineBytes)
+
+	lengths := []uint{}
+	for scanner.Scan() {
+		text := scanner.Text()
+		if len(lengths) == 0 {
+			text = strings.TrimPrefix(text, string(byteOrderMarkRune))
+		}
+		text = strings.TrimSuffix(text, "\r")
+		lengths = append(lengths, uint(utf8.RuneCountInString(text)))
+	}
+	if scanner.Err() != nil {
+		return nil
+	}
+	return lengths
+}
+
+// clampSonarColumns constrains a start offset and length to a line of lineLength characters.
+func clampSonarColumns(startColumn, length, lineLength uint) (start, end uint, emit bool) {
+	if lineLength == 0 {
+		return 0, 0, false
+	}
+
+	start = startColumn
+	if start > lineLength {
+		start = lineLength
+	}
+
+	end = start + length
+	if end > lineLength {
+		end = lineLength
+	}
+
+	// No forward span left: highlight the whole line instead of an invalid zero-width range.
+	if end <= start {
+		return 0, lineLength, true
+	}
+	return start, end, true
+}
+
+func parseSonarPrimaryLocation(results *wrappers.ScanResult, lineIndex *sonarLineIndex) wrappers.SonarLocation {
 	var auxLocation wrappers.SonarLocation
 	// fill the details in the primary Location
 	if len(results.ScanResultData.Nodes) > 0 {
 		auxLocation.FilePath = strings.TrimLeft(results.ScanResultData.Nodes[0].FileName, "/")
 		auxLocation.Message = html.UnescapeString(strings.ReplaceAll(results.ScanResultData.QueryName, "_", " "))
-		auxLocation.TextRange = parseSonarTextRange(results.ScanResultData.Nodes[0])
+		auxLocation.TextRange = parseSonarTextRange(results.ScanResultData.Nodes[0], lineIndex)
 	}
 	return auxLocation
 }
 
-func parseSonarSecondaryLocations(results *wrappers.ScanResult) []wrappers.SonarLocation {
+func parseSonarSecondaryLocations(results *wrappers.ScanResult, lineIndex *sonarLineIndex) []wrappers.SonarLocation {
 	var auxSecondaryLocations []wrappers.SonarLocation
 	// Traverse all the rest of the scan result nodes into secondary location of sonar
 	if len(results.ScanResultData.Nodes) >= 1 {
 		for _, node := range results.ScanResultData.Nodes[1:] {
+			filePath := strings.TrimLeft(node.FileName, "/")
+			if filePath == "" {
+				// filePath is mandatory on a secondary location too, so a node without one is skipped.
+				continue
+			}
+			textRange := parseSonarTextRange(node, lineIndex)
+			if textRange == nil {
+				// textRange is mandatory on secondary locations, so a node without a valid one is dropped.
+				continue
+			}
 			var auxSecondaryLocation wrappers.SonarLocation
-			auxSecondaryLocation.FilePath = strings.TrimLeft(node.FileName, "/")
+			auxSecondaryLocation.FilePath = filePath
 			auxSecondaryLocation.Message = html.UnescapeString(strings.ReplaceAll(results.ScanResultData.QueryName, "_", " "))
-			auxSecondaryLocation.TextRange = parseSonarTextRange(node)
+			auxSecondaryLocation.TextRange = textRange
 			auxSecondaryLocations = append(auxSecondaryLocations, auxSecondaryLocation)
 		}
 	}
 	return auxSecondaryLocations
 }
 
-func parseSonarTextRange(results *wrappers.ScanResultNode) wrappers.SonarTextRange {
-	var auxTextRange wrappers.SonarTextRange
-	auxTextRange.StartLine = results.Line
-	startColumn := getSastStartColumn(results.Column)
+// parseSonarTextRange maps a scan result node onto a Sonar text range, clamping columns to the verified line length.
+func parseSonarTextRange(results *wrappers.ScanResultNode, lineIndex *sonarLineIndex) *wrappers.SonarTextRange {
+	lineLength, status := lineIndex.resolveLine(results.FileName, results.Line)
+	if status == lineStatusLineMissing {
+		return nil
+	}
 
-	auxTextRange.StartColumn = startColumn
-	auxTextRange.EndColumn = startColumn + results.Length
+	auxTextRange := &wrappers.SonarTextRange{StartLine: results.Line}
+	if status != lineStatusOK {
+		return auxTextRange
+	}
 
-	if auxTextRange.StartColumn == auxTextRange.EndColumn {
-		auxTextRange.EndColumn++
+	startColumn, endColumn, emit := clampSonarColumns(getSastStartColumn(results.Column), results.Length, lineLength)
+	if !emit {
+		return auxTextRange
 	}
 
+	auxTextRange.StartColumn = startColumn
+	auxTextRange.EndColumn = endColumn
+
 	return auxTextRange
 }