From 1b189707050d361bf6471e50fe21f0b57c56fb2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Str=C3=B6mberg?= Date: Fri, 10 May 2024 13:10:02 -0400 Subject: [PATCH] Reduce memory usage by 30% through use of pointers (#211) * Use pointers wherever * Make it so that tests pass --- bincapz.go | 2 +- pkg/action/diff.go | 24 ++++++++++++------------ pkg/action/scan.go | 6 +++--- pkg/bincapz/bincapz.go | 33 ++++++++++++++++----------------- pkg/render/json.go | 4 ++-- pkg/render/markdown.go | 18 +++++++++++------- pkg/render/render.go | 4 ++-- pkg/render/simple.go | 8 ++++++-- pkg/render/stats.go | 6 +++--- pkg/render/terminal.go | 21 +++++++++++++-------- pkg/render/yaml.go | 4 ++-- pkg/report/report.go | 23 ++++++++++++----------- samples/macOS/clean/ls.json | 12 ++---------- samples/samples_test.go | 8 ++++---- 14 files changed, 89 insertions(+), 84 deletions(-) diff --git a/bincapz.go b/bincapz.go index 13851768..9ce03f5f 100644 --- a/bincapz.go +++ b/bincapz.go @@ -120,7 +120,7 @@ func main() { log.Fatal("failed", slog.Any("error", err)) } - err = renderer.Full(ctx, *res) + err = renderer.Full(ctx, res) if err != nil { log.Fatal("render failed", slog.Any("error", err)) } diff --git a/pkg/action/diff.go b/pkg/action/diff.go index dd82d1e3..f8dafe3d 100644 --- a/pkg/action/diff.go +++ b/pkg/action/diff.go @@ -14,7 +14,7 @@ import ( "github.com/chainguard-dev/clog" ) -func relFileReport(ctx context.Context, c Config, path string) (map[string]bincapz.FileReport, error) { +func relFileReport(ctx context.Context, c Config, path string) (map[string]*bincapz.FileReport, error) { fromPath := path fromConfig := c fromConfig.Renderer = nil @@ -23,7 +23,7 @@ func relFileReport(ctx context.Context, c Config, path string) (map[string]binca if err != nil { return nil, err } - fromRelPath := map[string]bincapz.FileReport{} + fromRelPath := map[string]*bincapz.FileReport{} for _, f := range fromReport.Files { if f.Skipped != "" || f.Error != "" { continue @@ -54,10 +54,10 @@ func Diff(ctx context.Context, c Config) (*bincapz.Report, error) { return nil, err } - d := bincapz.DiffReport{ - Added: map[string]bincapz.FileReport{}, - Removed: map[string]bincapz.FileReport{}, - Modified: map[string]bincapz.FileReport{}, + d := &bincapz.DiffReport{ + Added: map[string]*bincapz.FileReport{}, + Removed: map[string]*bincapz.FileReport{}, + Modified: map[string]*bincapz.FileReport{}, } // things that appear in the source @@ -73,9 +73,9 @@ func Diff(ctx context.Context, c Config) (*bincapz.Report, error) { continue } - rbs := bincapz.FileReport{ + rbs := &bincapz.FileReport{ Path: tr.Path, - Behaviors: map[string]bincapz.Behavior{}, + Behaviors: map[string]*bincapz.Behavior{}, PreviousRiskScore: fr.RiskScore, PreviousRiskLevel: fr.RiskLevel, RiskLevel: tr.RiskLevel, @@ -107,9 +107,9 @@ func Diff(ctx context.Context, c Config) (*bincapz.Report, error) { continue } - abs := bincapz.FileReport{ + abs := &bincapz.FileReport{ Path: tr.Path, - Behaviors: map[string]bincapz.Behavior{}, + Behaviors: map[string]*bincapz.Behavior{}, PreviousRiskScore: fr.RiskScore, PreviousRiskLevel: fr.RiskLevel, @@ -151,12 +151,12 @@ func Diff(ctx context.Context, c Config) (*bincapz.Report, error) { } // We think that this file moved from rpath to apath. - abs := bincapz.FileReport{ + abs := &bincapz.FileReport{ Path: tr.Path, PreviousRelPath: rpath, PreviousRelPathScore: score, - Behaviors: map[string]bincapz.Behavior{}, + Behaviors: map[string]*bincapz.Behavior{}, PreviousRiskScore: fr.RiskScore, PreviousRiskLevel: fr.RiskLevel, diff --git a/pkg/action/scan.go b/pkg/action/scan.go index 39b1049d..ee70e124 100644 --- a/pkg/action/scan.go +++ b/pkg/action/scan.go @@ -89,7 +89,7 @@ func recursiveScan(ctx context.Context, c Config) (*bincapz.Report, error) { logger := clog.FromContext(ctx) logger.Debug("scan", slog.Any("config", c)) r := &bincapz.Report{ - Files: map[string]bincapz.FileReport{}, + Files: map[string]*bincapz.FileReport{}, } if len(c.IgnoreTags) > 0 { r.Filter = strings.Join(c.IgnoreTags, ",") @@ -194,11 +194,11 @@ func processFile( if fr.RiskScore < c.MinFileScore { return nil } - if err := c.Renderer.File(ctx, *fr); err != nil { + if err := c.Renderer.File(ctx, fr); err != nil { return fmt.Errorf("render: %w", err) } } - r.Files[path] = *fr + r.Files[path] = fr return nil } diff --git a/pkg/bincapz/bincapz.go b/pkg/bincapz/bincapz.go index 4121a5cf..637453a9 100644 --- a/pkg/bincapz/bincapz.go +++ b/pkg/bincapz/bincapz.go @@ -27,14 +27,14 @@ type FileReport struct { Path string SHA256 string // compiler -> x - Error string `json:",omitempty" yaml:",omitempty"` - Skipped string `json:",omitempty" yaml:",omitempty"` - Meta map[string]string `json:",omitempty" yaml:",omitempty"` - Syscalls []string `json:",omitempty" yaml:",omitempty"` - Pledge []string `json:",omitempty" yaml:",omitempty"` - Capabilities []string `json:",omitempty" yaml:",omitempty"` - Behaviors map[string]Behavior `json:",omitempty" yaml:",omitempty"` - FilteredBehaviors int `json:",omitempty" yaml:",omitempty"` + Error string `json:",omitempty" yaml:",omitempty"` + Skipped string `json:",omitempty" yaml:",omitempty"` + Meta map[string]string `json:",omitempty" yaml:",omitempty"` + Syscalls []string `json:",omitempty" yaml:",omitempty"` + Pledge []string `json:",omitempty" yaml:",omitempty"` + Capabilities []string `json:",omitempty" yaml:",omitempty"` + Behaviors map[string]*Behavior `json:",omitempty" yaml:",omitempty"` + FilteredBehaviors int `json:",omitempty" yaml:",omitempty"` // The relative path we think this moved from. PreviousRelPath string `json:",omitempty" yaml:",omitempty"` @@ -43,23 +43,22 @@ type FileReport struct { PreviousRiskScore int `json:",omitempty" yaml:",omitempty"` PreviousRiskLevel string `json:",omitempty" yaml:",omitempty"` - RiskScore int - RiskLevel string `json:",omitempty" yaml:",omitempty"` - PackageRisk []string `json:",omitempty" yaml:",omitempty"` + RiskScore int + RiskLevel string `json:",omitempty" yaml:",omitempty"` IsBincapz bool `json:",omitempty" yaml:",omitempty"` } type DiffReport struct { - Added map[string]FileReport `json:",omitempty" yaml:",omitempty"` - Removed map[string]FileReport `json:",omitempty" yaml:",omitempty"` - Modified map[string]FileReport `json:",omitempty" yaml:",omitempty"` + Added map[string]*FileReport `json:",omitempty" yaml:",omitempty"` + Removed map[string]*FileReport `json:",omitempty" yaml:",omitempty"` + Modified map[string]*FileReport `json:",omitempty" yaml:",omitempty"` } type Report struct { - Files map[string]FileReport `json:",omitempty" yaml:",omitempty"` - Diff DiffReport `json:",omitempty" yaml:",omitempty"` - Filter string `json:",omitempty" yaml:",omitempty"` + Files map[string]*FileReport `json:",omitempty" yaml:",omitempty"` + Diff *DiffReport `json:",omitempty" yaml:",omitempty"` + Filter string `json:",omitempty" yaml:",omitempty"` } type IntMetric struct { diff --git a/pkg/render/json.go b/pkg/render/json.go index 568eaf85..75ac7661 100644 --- a/pkg/render/json.go +++ b/pkg/render/json.go @@ -20,11 +20,11 @@ func NewJSON(w io.Writer) JSON { return JSON{w: w} } -func (r JSON) File(_ context.Context, _ bincapz.FileReport) error { +func (r JSON) File(_ context.Context, _ *bincapz.FileReport) error { return nil } -func (r JSON) Full(_ context.Context, rep bincapz.Report) error { +func (r JSON) Full(_ context.Context, rep *bincapz.Report) error { j, err := json.MarshalIndent(rep, "", " ") if err != nil { return err diff --git a/pkg/render/markdown.go b/pkg/render/markdown.go index 3194d8db..e7804444 100644 --- a/pkg/render/markdown.go +++ b/pkg/render/markdown.go @@ -41,20 +41,24 @@ func matchFragmentLink(s string) string { return fmt.Sprintf("[%s](https://github.com/search?q=%s&type=code)", s, url.QueryEscape(s)) } -func (r Markdown) File(ctx context.Context, fr bincapz.FileReport) error { - markdownTable(ctx, &fr, r.w, tableConfig{Title: fmt.Sprintf("## %s [%s]", fr.Path, mdRisk(fr.RiskScore, fr.RiskLevel))}) +func (r Markdown) File(ctx context.Context, fr *bincapz.FileReport) error { + markdownTable(ctx, fr, r.w, tableConfig{Title: fmt.Sprintf("## %s [%s]", fr.Path, mdRisk(fr.RiskScore, fr.RiskLevel))}) return nil } -func (r Markdown) Full(ctx context.Context, rep bincapz.Report) error { +func (r Markdown) Full(ctx context.Context, rep *bincapz.Report) error { + if rep.Diff == nil { + return nil + } + for f, fr := range rep.Diff.Removed { fr := fr - markdownTable(ctx, &fr, r.w, tableConfig{Title: fmt.Sprintf("## Deleted: %s [%s]", f, mdRisk(fr.RiskScore, fr.RiskLevel)), DiffRemoved: true}) + markdownTable(ctx, fr, r.w, tableConfig{Title: fmt.Sprintf("## Deleted: %s [%s]", f, mdRisk(fr.RiskScore, fr.RiskLevel)), DiffRemoved: true}) } for f, fr := range rep.Diff.Added { fr := fr - markdownTable(ctx, &fr, r.w, tableConfig{Title: fmt.Sprintf("## Added: %s [%s]", f, mdRisk(fr.RiskScore, fr.RiskLevel)), DiffAdded: true}) + markdownTable(ctx, fr, r.w, tableConfig{Title: fmt.Sprintf("## Added: %s [%s]", f, mdRisk(fr.RiskScore, fr.RiskLevel)), DiffAdded: true}) } for f, fr := range rep.Diff.Modified { @@ -85,14 +89,14 @@ func (r Markdown) Full(ctx context.Context, rep bincapz.Report) error { } if added > 0 { - markdownTable(ctx, &fr, r.w, tableConfig{ + markdownTable(ctx, fr, r.w, tableConfig{ Title: fmt.Sprintf("### %d new behaviors", added), SkipRemoved: true, }) } if removed > 0 { - markdownTable(ctx, &fr, r.w, tableConfig{ + markdownTable(ctx, fr, r.w, tableConfig{ Title: fmt.Sprintf("### %d removed behaviors", removed), SkipAdded: true, }) diff --git a/pkg/render/render.go b/pkg/render/render.go index 0b621862..99597524 100644 --- a/pkg/render/render.go +++ b/pkg/render/render.go @@ -13,8 +13,8 @@ import ( // Renderer is a common interface for Renderers. type Renderer interface { - File(context.Context, bincapz.FileReport) error - Full(context.Context, bincapz.Report) error + File(context.Context, *bincapz.FileReport) error + Full(context.Context, *bincapz.Report) error } // New returns a new Renderer. diff --git a/pkg/render/simple.go b/pkg/render/simple.go index 767757c1..ec3f4430 100644 --- a/pkg/render/simple.go +++ b/pkg/render/simple.go @@ -20,7 +20,7 @@ func NewSimple(w io.Writer) Simple { return Simple{w: w} } -func (r Simple) File(_ context.Context, fr bincapz.FileReport) error { +func (r Simple) File(_ context.Context, fr *bincapz.FileReport) error { fmt.Fprintf(r.w, "# %s\n", fr.Path) bs := []string{} @@ -34,7 +34,11 @@ func (r Simple) File(_ context.Context, fr bincapz.FileReport) error { return nil } -func (r Simple) Full(_ context.Context, rep bincapz.Report) error { +func (r Simple) Full(_ context.Context, rep *bincapz.Report) error { + if rep.Diff == nil { + return nil + } + for f, fr := range rep.Diff.Removed { fmt.Fprintf(r.w, "--- missing: %s\n", f) diff --git a/pkg/render/stats.go b/pkg/render/stats.go index cd9759f0..15cbb963 100644 --- a/pkg/render/stats.go +++ b/pkg/render/stats.go @@ -8,7 +8,7 @@ import ( "github.com/chainguard-dev/bincapz/pkg/report" ) -func riskStatistics(files map[string]bincapz.FileReport) ([]bincapz.IntMetric, int, int) { +func riskStatistics(files map[string]*bincapz.FileReport) ([]bincapz.IntMetric, int, int) { riskMap := make(map[int][]string) riskStats := make(map[int]float64) @@ -49,12 +49,12 @@ func riskStatistics(files map[string]bincapz.FileReport) ([]bincapz.IntMetric, i return stats, total(), processedFiles } -func pkgStatistics(files map[string]bincapz.FileReport) ([]bincapz.StrMetric, int, int) { +func pkgStatistics(files map[string]*bincapz.FileReport) ([]bincapz.StrMetric, int, int) { numNamespaces := 0 pkgMap := make(map[string]int) pkg := make(map[string]float64) for _, rf := range files { - for _, namespace := range rf.PackageRisk { + for namespace := range rf.Behaviors { numNamespaces++ pkgMap[namespace]++ } diff --git a/pkg/render/terminal.go b/pkg/render/terminal.go index 6a96f54f..4d906ecc 100644 --- a/pkg/render/terminal.go +++ b/pkg/render/terminal.go @@ -23,7 +23,7 @@ var maxExampleCount = 8 type KeyedBehavior struct { Key string - Behavior bincapz.Behavior + Behavior *bincapz.Behavior } type tableConfig struct { @@ -91,8 +91,8 @@ func ShortRisk(s string) string { return short } -func (r Terminal) File(ctx context.Context, fr bincapz.FileReport) error { - renderTable(ctx, &fr, r.w, +func (r Terminal) File(ctx context.Context, fr *bincapz.FileReport) error { + renderTable(ctx, fr, r.w, tableConfig{ Title: fmt.Sprintf("%s %s", fr.Path, darkBrackets(decorativeRisk(fr.RiskScore, fr.RiskLevel))), }, @@ -100,10 +100,15 @@ func (r Terminal) File(ctx context.Context, fr bincapz.FileReport) error { return nil } -func (r Terminal) Full(ctx context.Context, rep bincapz.Report) error { +func (r Terminal) Full(ctx context.Context, rep *bincapz.Report) error { + // Non-diff files are handled on the fly by File() + if rep.Diff == nil { + return nil + } + for f, fr := range rep.Diff.Removed { fr := fr - renderTable(ctx, &fr, r.w, tableConfig{ + renderTable(ctx, fr, r.w, tableConfig{ Title: fmt.Sprintf("Deleted: %s %s", f, darkBrackets(decorativeRisk(fr.RiskScore, fr.RiskLevel))), DiffRemoved: true, }) @@ -111,7 +116,7 @@ func (r Terminal) Full(ctx context.Context, rep bincapz.Report) error { for f, fr := range rep.Diff.Added { fr := fr - renderTable(ctx, &fr, r.w, tableConfig{ + renderTable(ctx, fr, r.w, tableConfig{ Title: fmt.Sprintf("Added: %s %s", f, darkBrackets(decorativeRisk(fr.RiskScore, fr.RiskLevel))), DiffAdded: true, }) @@ -144,14 +149,14 @@ func (r Terminal) Full(ctx context.Context, rep bincapz.Report) error { } if added > 0 { - renderTable(ctx, &fr, r.w, tableConfig{ + renderTable(ctx, fr, r.w, tableConfig{ Title: color.HiWhiteString("+++ ADDED: %d behavior(s) +++", added), SkipRemoved: true, }) } if removed > 0 { - renderTable(ctx, &fr, r.w, tableConfig{ + renderTable(ctx, fr, r.w, tableConfig{ Title: color.HiWhiteString("--- REMOVED: %d behavior(s) ---", removed), SkipAdded: true, }) diff --git a/pkg/render/yaml.go b/pkg/render/yaml.go index 21e735aa..d59b7565 100644 --- a/pkg/render/yaml.go +++ b/pkg/render/yaml.go @@ -20,11 +20,11 @@ func NewYAML(w io.Writer) YAML { return YAML{w: w} } -func (r YAML) File(_ context.Context, _ bincapz.FileReport) error { +func (r YAML) File(_ context.Context, _ *bincapz.FileReport) error { return nil } -func (r YAML) Full(_ context.Context, rep bincapz.Report) error { +func (r YAML) Full(_ context.Context, rep *bincapz.Report) error { yaml, err := yaml.Marshal(rep) if err != nil { return err diff --git a/pkg/report/report.go b/pkg/report/report.go index f9dbabc4..01cb755b 100644 --- a/pkg/report/report.go +++ b/pkg/report/report.go @@ -287,7 +287,7 @@ func Generate(ctx context.Context, path string, mrs yara.MatchRules, ignoreTags Path: path, SHA256: ptCheck, Meta: map[string]string{}, - Behaviors: map[string]bincapz.Behavior{}, + Behaviors: map[string]*bincapz.Behavior{}, } pledges := []string{} @@ -295,10 +295,11 @@ func Generate(ctx context.Context, path string, mrs yara.MatchRules, ignoreTags syscalls := []string{} overallRiskScore := 0 riskCounts := map[int]int{} - packageRisks := []string{} + risk := 0 + key := "" for _, m := range mrs { - risk := behaviorRisk(m.Namespace, m.Rule, m.Tags) + risk = behaviorRisk(m.Namespace, m.Rule, m.Tags) if risk > overallRiskScore { overallRiskScore = risk } @@ -306,20 +307,22 @@ func Generate(ctx context.Context, path string, mrs yara.MatchRules, ignoreTags if risk < minScore { continue } - key := generateKey(m.Namespace, m.Rule) + key = generateKey(m.Namespace, m.Rule) ruleURL := generateRuleURL(m.Namespace, m.Rule) - packageRisks = append(packageRisks, key) - b := bincapz.Behavior{ + b := &bincapz.Behavior{ RiskScore: risk, RiskLevel: RiskLevels[risk], MatchStrings: matchStrings(m.Rule, m.Strings), RuleURL: ruleURL, } + k := "" + v := "" + for _, meta := range m.Metas { - k := meta.Identifier - v := fmt.Sprintf("%s", meta.Value) + k = meta.Identifier + v = fmt.Sprintf("%s", meta.Value) // Empty data is unusual, so just ignore it. if k == "" || v == "" { continue @@ -399,8 +402,7 @@ func Generate(ctx context.Context, path string, mrs yara.MatchRules, ignoreTags // If the existing description is longer, if len(existing.Description) < len(b.Description) { - existing.Description = b.Description - fr.Behaviors[key] = existing + fr.Behaviors[key].Description = b.Description } // TODO: If we match multiple rules within a single namespace, merge matchstrings @@ -418,7 +420,6 @@ func Generate(ctx context.Context, path string, mrs yara.MatchRules, ignoreTags fr.Capabilities = slices.Compact(caps) fr.RiskScore = overallRiskScore fr.RiskLevel = RiskLevels[fr.RiskScore] - fr.PackageRisk = packageRisks return fr, nil } diff --git a/samples/macOS/clean/ls.json b/samples/macOS/clean/ls.json index 1b7ed712..130f5ca4 100644 --- a/samples/macOS/clean/ls.json +++ b/samples/macOS/clean/ls.json @@ -51,15 +51,7 @@ } }, "RiskScore": 1, - "RiskLevel": "LOW", - "PackageRisk": [ - "env/TERM", - "fs/directory/traverse", - "fs/link/read", - "meta/format/macho", - "meta/program_name" - ] + "RiskLevel": "LOW" } - }, - "Diff": {} + } } diff --git a/samples/samples_test.go b/samples/samples_test.go index b6285eb5..034cf473 100644 --- a/samples/samples_test.go +++ b/samples/samples_test.go @@ -76,7 +76,7 @@ func TestJSON(t *testing.T) { t.Fatalf("scan failed: %v", err) } - if err := render.Full(ctx, *res); err != nil { + if err := render.Full(ctx, res); err != nil { t.Fatalf("full: %v", err) } @@ -140,7 +140,7 @@ func TestSimple(t *testing.T) { t.Fatalf("scan failed: %v", err) } - if err := simple.Full(ctx, *res); err != nil { + if err := simple.Full(ctx, res); err != nil { t.Fatalf("full: %v", err) } @@ -213,7 +213,7 @@ func TestDiff(t *testing.T) { t.Fatalf("diff failed: %v", err) } - if err := simple.Full(ctx, *res); err != nil { + if err := simple.Full(ctx, res); err != nil { t.Fatalf("full: %v", err) } @@ -287,7 +287,7 @@ func TestMarkdown(t *testing.T) { t.Fatalf("scan failed: %v", err) } - if err := simple.Full(ctx, *res); err != nil { + if err := simple.Full(ctx, res); err != nil { t.Fatalf("full: %v", err) }