fix: improve diff determinism for UPX files and file moves in general (#1265)

* fix: improve diff determinism for UPX files and file moves in general

Signed-off-by: egibs <[email protected]>

* use the decompressed files in handleDir as well

Signed-off-by: egibs <[email protected]>

---------

Signed-off-by: egibs <[email protected]>

Author: egibs

pkg/action/diff.go

@@ -7,10 +7,12 @@ import (
 	"context"
 	"fmt"
 	"log/slog"
+	"maps"
 	"os"
 	"path/filepath"
 	"regexp"
 	"slices"
+	"sort"
 	"strings"
 
 	"github.com/agext/levenshtein"
@@ -113,6 +115,37 @@ func relPath(from string, fr *malcontent.FileReport, isArchive bool, isImage boo
 	return rel, base, nil
 }
 
+// selectPrimaryFile selects a single file from a map of file reports in a deterministic way.
+// e.g., when a UPX-packed file is scanned, it produces the decompressed file
+// and preserves the original file (with a .~ suffix).
+func selectPrimaryFile(files map[string]*malcontent.FileReport) *malcontent.FileReport {
+	if len(files) == 0 {
+		return nil
+	}
+
+	keys := slices.Sorted(maps.Keys(files))
+
+	if i := slices.IndexFunc(keys, func(k string) bool {
+		return !strings.HasSuffix(k, ".~")
+	}); i >= 0 {
+		return files[keys[i]]
+	}
+
+	return files[keys[0]]
+}
+
+// isUPXBackup returns true if the path is a UPX backup file (.~ suffix)
+// and the corresponding decompressed file exists in the files map.
+func isUPXBackup(path string, files map[string]*malcontent.FileReport) bool {
+	if !strings.HasSuffix(path, ".~") {
+		return false
+	}
+
+	decompressed := strings.TrimSuffix(path, ".~")
+	_, exists := files[decompressed]
+	return exists
+}
+
 func relFileReport(ctx context.Context, c malcontent.Config, fromPath string, isImage bool) (map[string]*malcontent.FileReport, string, error) {
 	if ctx.Err() != nil {
 		return nil, "", ctx.Err()
@@ -283,15 +316,8 @@ func Diff(ctx context.Context, c malcontent.Config, _ *clog.Logger) (*malcontent
 	if shouldHandleDir {
 		handleDir(ctx, c, srcResult, destResult, d, archiveOrImage)
 	} else {
-		var srcFile, destFile *malcontent.FileReport
-		for _, fr := range srcResult.files {
-			srcFile = fr
-			break
-		}
-		for _, fr := range destResult.files {
-			destFile = fr
-			break
-		}
+		srcFile := selectPrimaryFile(srcResult.files)
+		destFile := selectPrimaryFile(destResult.files)
 		if srcFile != nil && destFile != nil {
 			removed := formatKey(srcResult, CleanPath(srcFile.Path, srcResult.tmpRoot))
 			added := formatKey(srcResult, CleanPath(destFile.Path, destResult.tmpRoot))
@@ -325,12 +351,12 @@ func handleDir(ctx context.Context, c malcontent.Config, src, dest ScanResult, d
 	srcFiles, destFiles := make(map[string]*malcontent.FileReport), make(map[string]*malcontent.FileReport)
 
 	for rel, fr := range src.files {
-		if rel != "" {
+		if rel != "" && !isUPXBackup(rel, src.files) {
 			srcFiles[rel] = fr
 		}
 	}
 	for rel, fr := range dest.files {
-		if rel != "" {
+		if rel != "" && !isUPXBackup(rel, dest.files) {
 			destFiles[rel] = fr
 		}
 	}
@@ -419,6 +445,11 @@ func handleFile(ctx context.Context, c malcontent.Config, fr, tr *malcontent.Fil
 		rbs.Behaviors = append(rbs.Behaviors, tb)
 	}
 
+	// Sort behaviors by ID for deterministic output
+	sort.Slice(rbs.Behaviors, func(i, j int) bool {
+		return rbs.Behaviors[i].ID < rbs.Behaviors[j].ID
+	})
+
 	if archiveOrImage {
 		rbs.Path = CleanPath(rbs.Path, "/private")
 		rbs.Path = formatKey(dest, CleanPath(rbs.Path, dest.tmpRoot))
@@ -598,6 +629,11 @@ func fileMove(ctx context.Context, c malcontent.Config, fr, tr *malcontent.FileR
 		}
 	}
 
+	// Sort behaviors by ID for deterministic output
+	sort.Slice(abs.Behaviors, func(i, j int) bool {
+		return abs.Behaviors[i].ID < abs.Behaviors[j].ID
+	})
+
 	if archiveOrImage {
 		abs.Path = CleanPath(abs.Path, "/private")
 		abs.PreviousPath = CleanPath(abs.PreviousPath, "/private")

tests/linux/2023.FreeDownloadManager/freedownloadmanager.sdiff

@@ -1,8 +1,8 @@
 *** changed (22 added, 1 removed): linux/2023.FreeDownloadManager/freedownloadmanager_infected_postinst
--c2/tool_transfer/os
 +anti-static/base64/exec
 +anti-static/base64/http_agent
 +anti-static/elf/base64
+-c2/tool_transfer/os
 +data/base64/external
 +data/embedded/base64_elf
 +data/embedded/base64_terms

tests/linux/2024.sbcl.market/sbcl.sdiff

@@ -1,5 +1,5 @@
 *** changed (3 added, 1 removed): linux/2024.sbcl.market/sbcl.dirty
--crypto/rc4
 +anti-static/elf/entropy
+-crypto/rc4
 +data/embedded/zstd
 +net/dns/txt

tests/macOS/clean/ls.sdiff.trigger_2

@@ -3,7 +3,7 @@
 -c2/tool_transfer/os
 -data/compression/lzma
 -discover/system/hostname
--net/url/embedded
--process/name_set
 +fs/directory/traverse
 +net/http
+-net/url/embedded
+-process/name_set