Repository: ossf/package-analysis (go)

Issue:
Remove entropy summaries and add individual entries to each literal
Currently, static analysis reports entropy statistics for each identifier and string literal. The summary is not super useful, it would be better to attach the entropy value to each individual string and identifier

Pull request:
Remove entropy summaries and record string literal and identifier entropy as part of parsing phase
Follow up to #838 as part of #836. Fixes #834 

High level changes:
1. move entropy calculations from obfuscation to parsing stage
2. fix bug in string entropy calculation
3. restructure tests for obfuscation.FileData
4. make GetLineLengths return integer slice rather than counted values
5. fix more json names
6. Don't use dict in `obfuscation.FileSignals` for suspicious identifiers

Still to do: improve unit tests for obfuscation.Analyze (e.g. URLs and IP address detection)

Patch:
diff --git a/internal/staticanalysis/basic_data.go b/internal/staticanalysis/basic_data.go
index f6b92b9..04c23ac 100644
--- a/internal/staticanalysis/basic_data.go
+++ b/internal/staticanalysis/basic_data.go
@@ -6,9 +6,6 @@ import (
 	"os/exec"
 	"strings"
 
-	"golang.org/x/exp/maps"
-	"golang.org/x/exp/slices"
-
 	"github.com/ossf/package-analysis/internal/log"
 	"github.com/ossf/package-analysis/internal/staticanalysis/externalcmd"
 	"github.com/ossf/package-analysis/internal/staticanalysis/linelengths"
@@ -37,26 +34,17 @@ type BasicFileData struct {
 	// Hash records the SHA256sum hash of the file.
 	Hash string `json:"hash"`
 
-	// LineLengthCounts records the count of lines of each length in the file,
+	// LineLengths records the counts of line lengths in the file,
 	// where a line is defined as all characters up to a newline.
-	LineLengthCounts valuecounts.ValueCounts `json:"line_length_counts"`
+	LineLengths valuecounts.ValueCounts `json:"line_lengths"`
 }
 
 func (bd BasicFileData) String() string {
-	// print line length counts in ascending order
-	lineLengths := maps.Keys(bd.LineLengthCounts)
-	slices.Sort(lineLengths)
-	lineLengthStrings := make([]string, 0, len(bd.LineLengthCounts))
-	for length := range lineLengths {
-		count := bd.LineLengthCounts[length]
-		lineLengthStrings = append(lineLengthStrings, fmt.Sprintf("length = %4d, count = %2d", length, count))
-	}
-
 	parts := []string{
 		fmt.Sprintf("file type: %v\n", bd.FileType),
 		fmt.Sprintf("size: %v\n", bd.Size),
 		fmt.Sprintf("hash: %v\n", bd.Hash),
-		fmt.Sprintf("line lengths:\n%s", strings.Join(lineLengthStrings, "\n")),
+		fmt.Sprintf("line lengths: %v\n", bd.LineLengths),
 	}
 	return strings.Join(parts, "\n")
 }
@@ -137,7 +125,6 @@ func GetBasicData(fileList []string, pathInArchive map[string]string) (*BasicPac
 
 	for index, filePath := range fileList {
 		archivePath := pathInArchive[filePath]
-
 		fileType := fileTypes[index]
 
 		var fileSize int64
@@ -156,19 +143,19 @@ func GetBasicData(fileList []string, pathInArchive map[string]string) (*BasicPac
 			fileHash = "sha256:" + hash
 		}
 
-		var lineLengthCounts valuecounts.ValueCounts
-		if lineLengths, err := linelengths.GetLineLengths(filePath, ""); err != nil {
-			log.Error("Error collecting line lengths", "path", archivePath, "error", err)
+		var lineLengths valuecounts.ValueCounts
+		if ll, err := linelengths.GetLineLengths(filePath, ""); err != nil {
+			log.Error("Error counting line lengths", "path", archivePath, "error", err)
 		} else {
-			lineLengthCounts = lineLengths
+			lineLengths = valuecounts.Count(ll)
 		}
 
 		result.Files = append(result.Files, BasicFileData{
-			Filename:         archivePath,
-			FileType:         fileType,
-			Size:             fileSize,
-			Hash:             fileHash,
-			LineLengthCounts: lineLengthCounts,
+			Filename:    archivePath,
+			FileType:    fileType,
+			Size:        fileSize,
+			Hash:        fileHash,
+			LineLengths: lineLengths,
 		})
 	}
 
diff --git a/internal/staticanalysis/linelengths/line_lengths.go b/internal/staticanalysis/linelengths/line_lengths.go
index 821c30e..bacbb18 100644
--- a/internal/staticanalysis/linelengths/line_lengths.go
+++ b/internal/staticanalysis/linelengths/line_lengths.go
@@ -5,13 +5,11 @@ import (
 	"io"
 	"os"
 	"strings"
-
-	"github.com/ossf/package-analysis/internal/utils/valuecounts"
 )
 
 /*
 GetLineLengths counts the number of characters on each line of a file or string,
-returning a map of length to the number of lines in the file with that length.
+returning a slice containing the length of each line in sequence.
 
 Lines are defined to be separated by newline ('\n') characters. If the newline
 character is preceded by a carriage return ('\r'), this will also be treated as
@@ -24,7 +22,7 @@ Note: there may not be much useful information to be gathered by distinguishing
 between line lengths when they get very long. It may be pragmatic to just report
 all lines above e.g. 64K as 64K long.
 */
-func GetLineLengths(filePath string, sourceString string) (valuecounts.ValueCounts, error) {
+func GetLineLengths(filePath string, sourceString string) ([]int, error) {
 	var reader *bufio.Reader
 	if len(filePath) > 0 {
 		file, err := os.Open(filePath)
@@ -38,7 +36,7 @@ func GetLineLengths(filePath string, sourceString string) (valuecounts.ValueCoun
 		reader = bufio.NewReader(strings.NewReader(sourceString))
 	}
 
-	lengths := valuecounts.ValueCounts{}
+	lengths := make([]int, 0)
 	for {
 		/* Normally bufio.Scanner would be more convenient to use here, however by default
 		it uses a fixed maximum buffer size (MaxScanTokenSize = 64 * 1024). Since some
@@ -61,7 +59,7 @@ func GetLineLengths(filePath string, sourceString string) (valuecounts.ValueCoun
 				}
 				l -= drop
 			}
-			lengths[l]++
+			lengths = append(lengths, l)
 		}
 
 		if readErr == io.EOF {
@@ -71,7 +69,7 @@ func GetLineLengths(filePath string, sourceString string) (valuecounts.ValueCoun
 
 	if len(lengths) == 0 {
 		// define the empty string to have a single empty line
-		lengths[0] = 1
+		lengths = append(lengths, 0)
 	}
 
 	return lengths, nil
diff --git a/internal/staticanalysis/obfuscation/analyze.go b/internal/staticanalysis/obfuscation/analyze.go
index 32e3c97..943628c 100644
--- a/internal/staticanalysis/obfuscation/analyze.go
+++ b/internal/staticanalysis/obfuscation/analyze.go
@@ -22,8 +22,6 @@ func Analyze(parseData []*parsing.SingleResult) Result {
 		case parsing.JavaScript:
 			signals := ComputeFileSignals(*fileData)
 			signals.Filename = fileData.Filename
-			// remove NaNs for JSON
-			RemoveNaNs(&signals)
 			result.Signals = append(result.Signals, signals)
 		}
 	}
diff --git a/internal/staticanalysis/obfuscation/file_signals.go b/internal/staticanalysis/obfuscation/file_signals.go
index e7d225c..e459b3f 100644
--- a/internal/staticanalysis/obfuscation/file_signals.go
+++ b/internal/staticanalysis/obfuscation/file_signals.go
@@ -2,12 +2,10 @@ package obfuscation
 
 import (
 	"fmt"
-	"math"
 	"strings"
+	"unicode/utf8"
 
 	"github.com/ossf/package-analysis/internal/staticanalysis/obfuscation/detections"
-	"github.com/ossf/package-analysis/internal/staticanalysis/obfuscation/stats"
-	"github.com/ossf/package-analysis/internal/staticanalysis/obfuscation/stringentropy"
 	"github.com/ossf/package-analysis/internal/staticanalysis/parsing"
 	"github.com/ossf/package-analysis/internal/staticanalysis/token"
 	"github.com/ossf/package-analysis/internal/utils"
@@ -17,161 +15,125 @@ import (
 // FileSignals holds information related to the presence of obfuscated code in a single file.
 type FileSignals struct {
 	// Filename is the path in the package
-	Filename string
-
-	// StringLengths counts the lengths of string literals in the file.
-	// If there is no count for a given string length, it means that there
-	// were no strings of that length in the file
-	StringLengths valuecounts.ValueCounts
-
-	// StringEntropySummary provides sample statistics for the set of entropy
-	// values calculated on each string literal. Character probabilities for the
-	// entropy calculation are estimated empirically from aggregated counts
-	// of characters across all string literals in the file.
-	StringEntropySummary stats.SampleStatistics
-
-	// CombinedStringEntropy is the entropy of the string obtained from
-	// concatenating all string literals in the file together. It may be used
-	// to normalise the values in StringEntropySummary.
-	CombinedStringEntropy float64
-
-	// IdentifierLengths counts the lengths of identifiers in the file.
-	// If there is no entry for a given length, it means that there
-	// were no identifiers of that length in the file
-	IdentifierLengths valuecounts.ValueCounts
-
-	// IdentifierEntropySummary provides sample statistics for the set of entropy
-	// values calculated on each identifier. Character probabilities for the
-	// entropy calculation are estimated empirically from aggregated counts
-	// of characters across all identifiers in the file.
-	IdentifierEntropySummary stats.SampleStatistics
-
-	// CombinedIdentifierEntropy is the entropy of the string obtained from
-	// concatenating all identifiers in the file together. It may be used to
-	// normalise the values in IdentifierEntropySummary.
-	CombinedIdentifierEntropy float64
-
-	// SuspiciousIdentifiers holds lists of identifiers that are deemed 'suspicious'
-	// (i.e. indicative of obfuscation) according to certain rules. The keys of the
-	// map are the rule names, and the values are the identifiers matching each rule.
-	SuspiciousIdentifiers map[string][]string
+	Filename string `json:"filename"`
+
+	// The following two variables respectively record how many string literals
+	// and identifiers in the file have a given length. The absence of a count
+	// for a particular lengths means that there were no symbols of that length
+	// in the file.
+	IdentifierLengths valuecounts.ValueCounts `json:"identifier_lengths"`
+	StringLengths     valuecounts.ValueCounts `json:"string_lengths"`
+
+	// SuspiciousIdentifiers holds identifiers that are deemed 'suspicious' (i.e.
+	// indicative of obfuscation) according to certain rules. Each entry contains
+	// the identifier name and the name of the first rule it was matched against.
+	SuspiciousIdentifiers []SuspiciousIdentifier `json:"suspicious_identifiers"`
+
+	// EscapedStrings contain string literals that contain large amount of escape
+	// characters, which may indicate obfuscation.
+	EscapedStrings []EscapedString `json:"escaped_strings"`
 
 	// Base64Strings holds a list of (substrings of) string literals found in the
 	// file that match a base64 regex pattern. This patten has a minimum matching
 	// length in order to reduce the number of false positives.
-	Base64Strings []string
+	Base64Strings []string `json:"base64_strings"`
+
+	// EmailAddresses contains any email addresses found in string literals
+	EmailAddresses []string `json:"email_addresses"`
 
 	// HexStrings holds a list of (substrings of) string literals found in the
 	// file that contain long (>8 digits) hexadecimal digit sequences.
-	HexStrings []string
-
-	// EscapedStrings contain string literals that contain large amount of escape
-	// characters, which may indicate obfuscation.
-	EscapedStrings []EscapedString
-
-	// URLs contains any urls (http or https) found in string literals
-	URLs []string
+	HexStrings []string `json:"hex_strings"`
 
 	// IPAddresses contains any IP addresses found in string literals
-	IPAddresses []string
+	IPAddresses []string `json:"ip_addresses"`
 
-	// EmailAddresses contains any email addresses found in string literals
-	EmailAddresses []string
+	// URLs contains any urls (http or https) found in string literals
+	URLs []string `json:"urls"`
 }
 
 func (s FileSignals) String() string {
 	parts := []string{
 		fmt.Sprintf("filename: %s", s.Filename),
-		fmt.Sprintf("string lengths: %v", s.StringLengths),
-		fmt.Sprintf("string entropy: %s", s.StringEntropySummary),
-		fmt.Sprintf("combined string entropy: %f", s.CombinedStringEntropy),
-
-		fmt.Sprintf("identifier lengths: %v", s.IdentifierLengths),
-		fmt.Sprintf("identifier entropy: %s", s.IdentifierEntropySummary),
-		fmt.Sprintf("combined identifier entropy: %f", s.CombinedIdentifierEntropy),
+		fmt.Sprintf("identifier length counts: %v", s.IdentifierLengths),
+		fmt.Sprintf("string length counts: %v", s.StringLengths),
 
 		fmt.Sprintf("suspicious identifiers: %v", s.SuspiciousIdentifiers),
+		fmt.Sprintf("escaped strings: %v", s.EscapedStrings),
 		fmt.Sprintf("potential base64 strings: %v", s.Base64Strings),
 		fmt.Sprintf("hex strings: %v", s.HexStrings),
-		fmt.Sprintf("escaped strings: %v", s.EscapedStrings),
+		fmt.Sprintf("email addresses: %v", s.EmailAddresses),
+		fmt.Sprintf("hex strings: %v", s.HexStrings),
+		fmt.Sprintf("IP addresses: %v", s.IPAddresses),
+		fmt.Sprintf("URLs: %v", s.URLs),
 	}
 	return strings.Join(parts, "\n")
 }
 
 type EscapedString struct {
-	RawLiteral       string
-	LevenshteinRatio float64
+	RawLiteral       string  `json:"raw_literal"`
+	LevenshteinRatio float64 `json:"levenshtein_ratio"`
 }
 
-/*
-characterAnalysis performs analysis on a collection of string symbols
-
-This function returns:
-
-  - Counts of symbol (string) lengths,
-  - Stats summary of symbol (string) entropies,
-  - Entropy of all symbols concatenated together.
-*/
-func characterAnalysis(symbols []string) (
-	lengthCounts map[int]int,
-	entropySummary stats.SampleStatistics,
-	combinedEntropy float64,
-) {
-	// measure character probabilities by looking at entire set of strings
-	characterProbs := stringentropy.CharacterProbabilities(symbols)
-
-	var entropies []float64
-	var lengths []int
+// SuspiciousIdentifier is an identifier that matches a specific rule intended
+// to pick out (potentially) suspicious names. Name stores the actual identifier,
+// and Rule holds the rule that the identifier matched against.
+type SuspiciousIdentifier struct {
+	Name string `json:"name"`
+	Rule string `json:"reason"`
+}
+
+// countLengths returns a map containing the aggregated lengths
+// of each of the strings in the input list
+func countLengths(symbols []string) valuecounts.ValueCounts {
+	lengths := make([]int, 0, len(symbols))
 	for _, s := range symbols {
-		entropies = append(entropies, stringentropy.CalculateEntropy(s, characterProbs))
-		lengths = append(lengths, len(s))
+		lengths = append(lengths, utf8.RuneCountInString(s))
 	}
 
-	lengthCounts = stats.CountDistinct(lengths)
-	entropySummary = stats.Summarise(entropies)
-	combinedEntropy = stringentropy.CalculateEntropy(strings.Join(symbols, ""), nil)
-	return
+	return valuecounts.Count(lengths)
 }
 
-/*
-ComputeFileSignals creates a FileSignals object based on the data obtained from ParseFile
-for a given file. These signals may be useful to determine whether the code is obfuscated.
-*/
+// ComputeFileSignals creates a FileSignals object based on the parsing data obtained from
+// a given file. These signals may be useful to determine whether the code is obfuscated.
 func ComputeFileSignals(parseData parsing.SingleResult) FileSignals {
-	signals := FileSignals{}
-
-	literals := utils.Transform(parseData.StringLiterals, func(s token.String) string { return s.Value })
-	signals.StringLengths, signals.StringEntropySummary, signals.CombinedStringEntropy = characterAnalysis(literals)
-
 	identifierNames := utils.Transform(parseData.Identifiers, func(i token.Identifier) string { return i.Name })
-	signals.IdentifierLengths, signals.IdentifierEntropySummary, signals.CombinedIdentifierEntropy = characterAnalysis(identifierNames)
+	stringLiterals := utils.Transform(parseData.StringLiterals, func(s token.String) string { return s.Value })
+
+	identifierLengths := countLengths(identifierNames)
+	stringLengths := countLengths(stringLiterals)
+
+	signals := FileSignals{
+		IdentifierLengths:     identifierLengths,
+		StringLengths:         stringLengths,
+		Base64Strings:         []string{},
+		HexStrings:            []string{},
+		EscapedStrings:        []EscapedString{},
+		SuspiciousIdentifiers: []SuspiciousIdentifier{},
+		URLs:                  []string{},
+		IPAddresses:           []string{},
+		EmailAddresses:        []string{},
+	}
 
-	signals.SuspiciousIdentifiers = map[string][]string{}
-	for ruleName, pattern := range detections.SuspiciousIdentifierPatterns {
-		signals.SuspiciousIdentifiers[ruleName] = []string{}
-		for _, name := range identifierNames {
+	for _, name := range identifierNames {
+		for rule, pattern := range detections.SuspiciousIdentifierPatterns {
 			if pattern.MatchString(name) {
-				signals.SuspiciousIdentifiers[ruleName] = append(signals.SuspiciousIdentifiers[ruleName], name)
+				signals.SuspiciousIdentifiers = append(signals.SuspiciousIdentifiers, SuspiciousIdentifier{name, rule})
+				break // don't bother searching for multiple matching rules
 			}
 		}
 	}
 
-	signals.Base64Strings = []string{}
-	signals.HexStrings = []string{}
-	signals.EscapedStrings = []EscapedString{}
-	signals.URLs = []string{}
-	signals.IPAddresses = []string{}
-	signals.EmailAddresses = []string{}
-	for _, s := range parseData.StringLiterals {
-		signals.Base64Strings = append(signals.Base64Strings, detections.FindBase64Substrings(s.Value)...)
-		signals.HexStrings = append(signals.HexStrings, detections.FindHexSubstrings(s.Value)...)
-		signals.URLs = append(signals.URLs, detections.FindURLs(s.Value)...)
-		signals.IPAddresses = append(signals.IPAddresses, detections.FindIPAddresses(s.Value)...)
-		signals.EmailAddresses = append(signals.EmailAddresses, detections.FindEmailAddresses(s.Value)...)
-		if detections.IsHighlyEscaped(s, 8, 0.25) {
+	for _, sl := range parseData.StringLiterals {
+		signals.Base64Strings = append(signals.Base64Strings, detections.FindBase64Substrings(sl.Value)...)
+		signals.HexStrings = append(signals.HexStrings, detections.FindHexSubstrings(sl.Value)...)
+		signals.URLs = append(signals.URLs, detections.FindURLs(sl.Value)...)
+		signals.IPAddresses = append(signals.IPAddresses, detections.FindIPAddresses(sl.Value)...)
+		signals.EmailAddresses = append(signals.EmailAddresses, detections.FindEmailAddresses(sl.Value)...)
+		if detections.IsHighlyEscaped(sl, 8, 0.25) {
 			escapedString := EscapedString{
-				RawLiteral:       s.Raw,
-				LevenshteinRatio: detections.LevenshteinRatio(s),
+				RawLiteral:       sl.Raw,
+				LevenshteinRatio: detections.LevenshteinRatio(sl),
 			}
 			signals.EscapedStrings = append(signals.EscapedStrings, escapedString)
 		}
@@ -179,16 +141,3 @@ func ComputeFileSignals(parseData parsing.SingleResult) FileSignals {
 
 	return signals
 }
-
-// RemoveNaNs replaces all NaN values in this object with zeros.
-func RemoveNaNs(s *FileSignals) {
-	s.StringEntropySummary = s.StringEntropySummary.ReplaceNaNs(0)
-	s.IdentifierEntropySummary = s.IdentifierEntropySummary.ReplaceNaNs(0)
-
-	if math.IsNaN(s.CombinedStringEntropy) {
-		s.CombinedStringEntropy = 0.0
-	}
-	if math.IsNaN(s.CombinedIdentifierEntropy) {
-		s.CombinedIdentifierEntropy = 0.0
-	}
-}
diff --git a/internal/staticanalysis/obfuscation/stats/sample_statistics.go b/internal/staticanalysis/obfuscation/stats/sample_statistics.go
index b20de95..36bae5e 100644
--- a/internal/staticanalysis/obfuscation/stats/sample_statistics.go
+++ b/internal/staticanalysis/obfuscation/stats/sample_statistics.go
@@ -8,6 +8,7 @@ import (
 	"golang.org/x/exp/slices"
 
 	"github.com/ossf/package-analysis/internal/utils"
+	"github.com/ossf/package-analysis/internal/utils/valuecounts"
 )
 
 type RealNumber interface {
@@ -209,8 +210,8 @@ func Summarise[T RealNumber](sample []T) SampleStatistics {
 	return SampleStatistics{Size: l, Mean: m, Variance: v, Skewness: s, Quartiles: q}
 }
 
-func CountDistinct[T constraints.Integer](sample []T) map[T]int {
-	counts := make(map[T]int, 0)
+func CountDistinct(sample []int) valuecounts.ValueCounts {
+	counts := valuecounts.New()
 	for _, t := range sample {
 		counts[t] += 1
 	}
diff --git a/internal/staticanalysis/obfuscation/stringentropy/string_entropy.go b/internal/staticanalysis/obfuscation/stringentropy/string_entropy.go
index 0d16771..48549d2 100644
--- a/internal/staticanalysis/obfuscation/stringentropy/string_entropy.go
+++ b/internal/staticanalysis/obfuscation/stringentropy/string_entropy.go
@@ -2,6 +2,7 @@ package stringentropy
 
 import (
 	"math"
+	"unicode/utf8"
 )
 
 /*
@@ -23,23 +24,22 @@ the entropy approaches 0.
 
 Reference: https://link.springer.com/chapter/10.1007/978-3-642-10509-8_19
 */
-func CalculateEntropy(s string, prob *map[rune]float64) float64 {
+func CalculateEntropy(s string, prob map[rune]float64) float64 {
 	if len(s) == 0 {
 		return 0
 	}
 
-	counts := CharacterCounts([]string{s})
-
 	if prob == nil {
-		prob = CharacterProbabilitiesFromCounts(counts)
+		counts, sumCounts := CharacterCounts([]string{s})
+		prob = characterProbabilitiesFromCounts(counts, sumCounts)
 	}
 
 	entropy := 0.0
-
-	// can't iterate over string as we need to iterate over distinct chars
-	for char := range *counts {
-		p := (*prob)[char]
-		entropy -= p * math.Log(p)
+	for _, char := range s {
+		p := prob[char]
+		if p > 0 {
+			entropy -= p * math.Log(p)
+		}
 	}
 
 	return entropy
@@ -48,7 +48,7 @@ func CalculateEntropy(s string, prob *map[rune]float64) float64 {
 /*
 CalculateNormalisedEntropy returns the string entropy normalised by the log of the length of the string.
 This quantity is used because for log(N) is the maximum possible entropy out of all strings with length N,
-where N > 0. If the string has one or fewer characters, the ratio is defined to be 1.
+where N > 0. Special cases are empty strings (0) and single character strings (1).
 As a formula:
 
 	E_n(S) := {
@@ -59,44 +59,43 @@ As a formula:
 */
 // TODO does this make sense when a general probability structure is used?
 // TODO calculate max string entropy for a given set of character counts.
-func CalculateNormalisedEntropy(s string, prob *map[rune]float64) float64 {
-	switch len(s) {
+func CalculateNormalisedEntropy(s string, prob map[rune]float64) float64 {
+	length := utf8.RuneCountInString(s)
+	switch length {
 	case 0:
 		return 0
 	case 1:
 		return 1
 	default:
-		return CalculateEntropy(s, prob) / math.Log(float64(len(s)))
+		return CalculateEntropy(s, prob) / math.Log(float64(length))
 	}
 }
 
-func CharacterCounts(strs []string) *map[rune]int {
+// CharacterCounts computes a map of character (rune) to number of occurrences
+// in the input strings
+func CharacterCounts(strs []string) (map[rune]int, int64) {
 	counts := make(map[rune]int)
+	var sumCounts int64 = 0
 	for _, s := range strs {
 		for _, b := range s {
-			// if b is not in map, the read value will be zero, which is what we want
-			counts[b] = counts[b] + 1
+			counts[b] += 1
+			sumCounts += 1
 		}
 	}
-	return &counts
+	return counts, sumCounts
 }
 
-func CharacterProbabilitiesFromCounts(counts *map[rune]int) *map[rune]float64 {
-	total := 0
-	for _, count := range *counts {
-		total += count
-	}
-
-	prob := make(map[rune]float64, len(*counts))
-	for char, count := range *counts {
-		prob[char] = float64(count) / float64(total)
-	}
-	return &prob
+// CharacterProbabilities computes a map of character (rune) to
+// frequency/probability of occurrence in the input strings
+func CharacterProbabilities(strs []string) map[rune]float64 {
+	counts, sumCounts := CharacterCounts(strs)
+	return characterProbabilitiesFromCounts(counts, sumCounts)
 }
 
-// CharacterProbabilities is just a convenience function that chains together CharacterCounts
-// and CharacterProbabilitiesFromCounts.
-func CharacterProbabilities(strs []string) *map[rune]float64 {
-	counts := CharacterCounts(strs)
-	return CharacterProbabilitiesFromCounts(counts)
+func characterProbabilitiesFromCounts(counts map[rune]int, sumCounts int64) map[rune]float64 {
+	prob := make(map[rune]float64, len(counts))
+	for char, count := range counts {
+		prob[char] = float64(count) / float64(sumCounts)
+	}
+	return prob
 }
diff --git a/internal/staticanalysis/parsing/analyze.go b/internal/staticanalysis/parsing/analyze.go
index 4dbc956..82039eb 100644
--- a/internal/staticanalysis/parsing/analyze.go
+++ b/internal/staticanalysis/parsing/analyze.go
@@ -6,6 +6,7 @@ import (
 	"strconv"
 
 	"github.com/ossf/package-analysis/internal/staticanalysis/externalcmd"
+	"github.com/ossf/package-analysis/internal/staticanalysis/obfuscation/stringentropy"
 	"github.com/ossf/package-analysis/internal/staticanalysis/token"
 )
 
@@ -50,10 +51,38 @@ func processJsData(filename string, fileData singleParseData) *SingleResult {
 	for _, c := range fileData.Comments {
 		result.Comments = append(result.Comments, token.Comment{Value: c.Data})
 	}
-
 	return result
 }
 
+// computeEntropy populates entropy values for identifiers and string literals.
+// Character frequency distribution is estimated by aggregating character counts
+// in across all identifiers and string literals respectively in the package.
+func computeEntropy(parseResults []*SingleResult) {
+	var strings []string
+	var identifiers []string
+
+	for _, result := range parseResults {
+		for _, sl := range result.StringLiterals {
+			strings = append(strings, sl.Value)
+		}
+		for _, id := range result.Identifiers {
+			identifiers = append(identifiers, id.Name)
+		}
+	}
+
+	stringLiteralCharDistribution := stringentropy.CharacterProbabilities(strings)
+	identifierCharDistribution := stringentropy.CharacterProbabilities(identifiers)
+
+	for _, result := range parseResults {
+		for _, sl := range result.StringLiterals {
+			sl.Entropy = stringentropy.CalculateEntropy(sl.Value, stringLiteralCharDistribution)
+		}
+		for _, id := range result.Identifiers {
+			id.Entropy = stringentropy.CalculateEntropy(id.Name, identifierCharDistribution)
+		}
+	}
+}
+
 /*
 Analyze (parsing.Analyze) parses the specified list of files using all supported parsers
 and returns a list of parsing.SingleResult, each of which holds information about source code
@@ -91,5 +120,7 @@ func Analyze(parserConfig ParserConfig, input externalcmd.Input, printDebug bool
 		packageResult = append(packageResult, processJsData(filename, jsData))
 	}
 
+	computeEntropy(packageResult)
+
 	return packageResult, nil
 }
diff --git a/internal/staticanalysis/token/tokens.go b/internal/staticanalysis/token/tokens.go
index 9c9d122..b10a9a7 100644
--- a/internal/staticanalysis/token/tokens.go
+++ b/internal/staticanalysis/token/tokens.go
@@ -1,8 +1,9 @@
 package token
 
 type Identifier struct {
-	Name string
-	Type IdentifierType
+	Name    string
+	Type    IdentifierType
+	Entropy float64
 }
 
 type Comment struct {
@@ -10,8 +11,9 @@ type Comment struct {
 }
 
 type String struct {
-	Value string
-	Raw   string
+	Value   string
+	Raw     string
+	Entropy float64
 }
 
 type Int struct {
diff --git a/internal/utils/valuecounts/value_counts.go b/internal/utils/valuecounts/value_counts.go
index cc93262..a1d2005 100644
--- a/internal/utils/valuecounts/value_counts.go
+++ b/internal/utils/valuecounts/value_counts.go
@@ -3,6 +3,7 @@ package valuecounts
 import (
 	"encoding/json"
 	"fmt"
+	"strings"
 
 	"golang.org/x/exp/maps"
 	"golang.org/x/exp/slices"
@@ -26,6 +27,25 @@ func New() ValueCounts {
 	return ValueCounts{}
 }
 
+// Count produces a new ValueCounts by counting repetitions of values in the input data
+func Count(data []int) ValueCounts {
+	vc := New()
+	for _, value := range data {
+		vc[value] += 1
+	}
+	return vc
+}
+
+// String() returns a string representation of this ValueCounts
+// with values sorted in ascending order
+func (vc ValueCounts) String() string {
+	pairStrings := make([]string, 0, len(vc))
+	for _, pair := range vc.ToPairs() {
+		pairStrings = append(pairStrings, fmt.Sprintf("%d: %d", pair.Value, pair.Count))
+	}
+	return "[" + strings.Join(pairStrings, ", ") + " ]"
+}
+
 // ToPairs converts this ValueCounts into a list of (value, count) pairs.
 // The values are sorted in increasing order so that the output is deterministic.
 // If this ValueCounts is empty, returns an empty slice.


Test patch:
diff --git a/internal/staticanalysis/linelengths/line_lengths_test.go b/internal/staticanalysis/linelengths/line_lengths_test.go
index b976cbe..9c48226 100644
--- a/internal/staticanalysis/linelengths/line_lengths_test.go
+++ b/internal/staticanalysis/linelengths/line_lengths_test.go
@@ -3,15 +3,13 @@ package linelengths
 import (
 	"reflect"
 	"testing"
-
-	"github.com/ossf/package-analysis/internal/utils/valuecounts"
 )
 
 func TestSourceStringLineLengths(t *testing.T) {
 	tests := []struct {
 		name    string
 		source  string
-		want    valuecounts.ValueCounts
+		want    []int
 		wantErr bool
 	}{
 		{
@@ -23,39 +21,39 @@ Three
 Four
 Five
 `,
-			want:    map[int]int{0: 1, 3: 2, 4: 2, 5: 1},
+			want:    []int{0, 3, 3, 5, 4, 4},
 			wantErr: false,
 		},
 		{
 			name:    "test simple single line",
 			source:  `One Two Three Four Five`,
-			want:    map[int]int{23: 1},
+			want:    []int{23},
 			wantErr: false,
 		},
 		{
 			name:    "test empty string",
 			source:  ``,
-			want:    map[int]int{0: 1},
+			want:    []int{0},
 			wantErr: false,
 		},
 		{
 			name:    "test single char",
 			source:  "a",
-			want:    map[int]int{1: 1},
+			want:    []int{1},
 			wantErr: false,
 		},
 		{
 			name: "test empty newline",
 			source: `
 `,
-			want:    map[int]int{0: 1},
+			want:    []int{0},
 			wantErr: false,
 		},
 
 		{
 			name:    "test carriage return",
 			source:  "\r\n",
-			want:    map[int]int{0: 1},
+			want:    []int{0},
 			wantErr: false,
 		},
 	}
diff --git a/internal/staticanalysis/obfuscation/file_signals_test.go b/internal/staticanalysis/obfuscation/file_signals_test.go
index b4f2e71..f6f22ba 100644
--- a/internal/staticanalysis/obfuscation/file_signals_test.go
+++ b/internal/staticanalysis/obfuscation/file_signals_test.go
@@ -2,20 +2,17 @@ package obfuscation
 
 import (
 	"reflect"
-	"strings"
 	"testing"
 
-	"github.com/ossf/package-analysis/internal/log"
-	"github.com/ossf/package-analysis/internal/staticanalysis/obfuscation/stats"
-	"github.com/ossf/package-analysis/internal/staticanalysis/obfuscation/stringentropy"
 	"github.com/ossf/package-analysis/internal/staticanalysis/parsing"
 	"github.com/ossf/package-analysis/internal/staticanalysis/token"
-	"github.com/ossf/package-analysis/internal/utils"
+	"github.com/ossf/package-analysis/internal/utils/valuecounts"
 )
 
 type fileSignalsTestCase struct {
-	name      string
-	parseData parsing.SingleResult
+	name            string
+	parseData       parsing.SingleResult
+	expectedSignals FileSignals
 }
 
 var fileSignalsTestCases = []fileSignalsTestCase{
@@ -31,6 +28,17 @@ var fileSignalsTestCases = []fileSignalsTestCase{
 			IntLiterals:   []token.Int{},
 			FloatLiterals: []token.Float{},
 		},
+		expectedSignals: FileSignals{
+			StringLengths:         valuecounts.ValueCounts{5: 1},
+			IdentifierLengths:     valuecounts.ValueCounts{1: 1},
+			SuspiciousIdentifiers: []SuspiciousIdentifier{{Name: "a", Rule: "single"}},
+			EscapedStrings:        []EscapedString{},
+			Base64Strings:         []string{},
+			EmailAddresses:        []string{},
+			HexStrings:            []string{},
+			IPAddresses:           []string{},
+			URLs:                  []string{},
+		},
 	},
 	{
 		name: "simple 2",
@@ -52,70 +60,32 @@ var fileSignalsTestCases = []fileSignalsTestCase{
 			},
 			FloatLiterals: []token.Float{},
 		},
+		expectedSignals: FileSignals{
+			StringLengths:     valuecounts.ValueCounts{5: 2},
+			IdentifierLengths: valuecounts.ValueCounts{1: 3, 4: 1},
+			SuspiciousIdentifiers: []SuspiciousIdentifier{
+				{Name: "a", Rule: "single"},
+				{Name: "b", Rule: "single"},
+				{Name: "c", Rule: "single"},
+			},
+			EscapedStrings: []EscapedString{},
+			Base64Strings:  []string{},
+			EmailAddresses: []string{},
+			HexStrings:     []string{},
+			IPAddresses:    []string{},
+			URLs:           []string{},
+		},
 	},
 }
 
-func symbolEntropySummary(symbols []string) stats.SampleStatistics {
-	probs := stringentropy.CharacterProbabilities(symbols)
-	entropies := utils.Transform(symbols, func(s string) float64 { return stringentropy.CalculateEntropy(s, probs) })
-	return stats.Summarise(entropies)
-}
-
-func symbolLengthCounts(symbols []string) map[int]int {
-	lengths := utils.Transform(symbols, func(s string) int { return len(s) })
-	return stats.CountDistinct(lengths)
-}
-
-func compareSummary(t *testing.T, name string, expected, actual stats.SampleStatistics) {
-	t.Helper()
-	if !expected.Equals(actual, 1e-4) {
-		t.Errorf("%s summary did not match.\nExpected: %v\nActual: %v\n", name, expected, actual)
-	}
-}
-
-func compareCounts(t *testing.T, name string, expected, actual map[int]int) {
-	t.Helper()
-	if !reflect.DeepEqual(expected, actual) {
-		t.Errorf("%s summary did not match.\nExpected: %v\nActual: %v\n", name, expected, actual)
-	}
-}
-
-func testSignals(t *testing.T, signals FileSignals, stringLiterals []token.String, identifiers []token.Identifier) {
-	t.Helper()
-	literals := utils.Transform(stringLiterals, func(s token.String) string { return s.Value })
-	identifierNames := utils.Transform(identifiers, func(i token.Identifier) string { return i.Name })
-	expectedStringEntropySummary := symbolEntropySummary(literals)
-	expectedStringLengthSummary := symbolLengthCounts(literals)
-	expectedIdentifierEntropySummary := symbolEntropySummary(identifierNames)
-	expectedIdentifierLengthSummary := symbolLengthCounts(identifierNames)
-
-	compareSummary(t, "String literal entropy", expectedStringEntropySummary, signals.StringEntropySummary)
-	compareCounts(t, "String literal lengths", expectedStringLengthSummary, signals.StringLengths)
-	compareSummary(t, "Identifier entropy", expectedIdentifierEntropySummary, signals.IdentifierEntropySummary)
-	compareCounts(t, "Identifier lengths", expectedIdentifierLengthSummary, signals.IdentifierLengths)
-
-	expectedStringCombinedEntropy := stringentropy.CalculateEntropy(strings.Join(literals, ""), nil)
-	if !utils.FloatEquals(expectedStringCombinedEntropy, signals.CombinedStringEntropy, 1e-4) {
-		t.Errorf("Combined string entropy: expected %.3f, actual %.3f",
-			expectedStringCombinedEntropy, signals.CombinedStringEntropy)
-	}
-
-	expectedIdentifierCombinedEntropy := stringentropy.CalculateEntropy(strings.Join(identifierNames, ""), nil)
-	if !utils.FloatEquals(expectedIdentifierCombinedEntropy, signals.CombinedIdentifierEntropy, 1e-4) {
-		t.Errorf("Combined identifier entropy: expected %.3f, actual %.3f",
-			expectedIdentifierCombinedEntropy, signals.CombinedIdentifierEntropy)
-	}
-}
-
-func init() {
-	log.Initialize("")
-}
-
 func TestComputeSignals(t *testing.T) {
 	for _, test := range fileSignalsTestCases {
 		t.Run(test.name, func(t *testing.T) {
 			signals := ComputeFileSignals(test.parseData)
-			testSignals(t, signals, test.parseData.StringLiterals, test.parseData.Identifiers)
+			if !reflect.DeepEqual(signals, test.expectedSignals) {
+				t.Errorf("actual signals did not match expected\n"+
+					"== want ==\n%v\n== got ==\n%v\n======", test.expectedSignals, signals)
+			}
 		})
 	}
 }
diff --git a/internal/staticanalysis/obfuscation/stats/sample_statistics_test.go b/internal/staticanalysis/obfuscation/stats/sample_statistics_test.go
index fb9e7f5..315fa81 100644
--- a/internal/staticanalysis/obfuscation/stats/sample_statistics_test.go
+++ b/internal/staticanalysis/obfuscation/stats/sample_statistics_test.go
@@ -4,6 +4,8 @@ import (
 	"math"
 	"reflect"
 	"testing"
+
+	"github.com/ossf/package-analysis/internal/utils/valuecounts"
 )
 
 func TestSummary(t *testing.T) {
@@ -117,7 +119,7 @@ func TestSummary7(t *testing.T) {
 func TestCountDistinct1(t *testing.T) {
 	data := []int{1, 2, 3, 4, 3, 2, 1, 2}
 	actual := CountDistinct(data)
-	expected := map[int]int{1: 2, 2: 3, 3: 2, 4: 1}
+	expected := valuecounts.ValueCounts{1: 2, 2: 3, 3: 2, 4: 1}
 	if !reflect.DeepEqual(actual, expected) {
 		t.Errorf("Expected counts: %v\nactual counts %v\n", expected, actual)
 	}
@@ -126,7 +128,7 @@ func TestCountDistinct1(t *testing.T) {
 func TestCountDistinct2(t *testing.T) {
 	data := []int{1}
 	actual := CountDistinct(data)
-	expected := map[int]int{1: 1}
+	expected := valuecounts.ValueCounts{1: 1}
 	if !reflect.DeepEqual(actual, expected) {
 		t.Errorf("Expected counts: %v\nactual counts %v\n", expected, actual)
 	}
@@ -135,7 +137,7 @@ func TestCountDistinct2(t *testing.T) {
 func TestCountDistinct3(t *testing.T) {
 	data := []int{}
 	actual := CountDistinct(data)
-	expected := map[int]int{}
+	expected := valuecounts.ValueCounts{}
 	if !reflect.DeepEqual(actual, expected) {
 		t.Errorf("Expected counts: %v\nactual counts %v\n", expected, actual)
 	}
diff --git a/internal/staticanalysis/obfuscation/stringentropy/string_entropy_test.go b/internal/staticanalysis/obfuscation/stringentropy/string_entropy_test.go
index 2a0466d..334e491 100644
--- a/internal/staticanalysis/obfuscation/stringentropy/string_entropy_test.go
+++ b/internal/staticanalysis/obfuscation/stringentropy/string_entropy_test.go
@@ -21,7 +21,7 @@ func TestStringEntropy(t *testing.T) {
 		{"abcdefghij", math.Log(10)},
 		{"aaa", 0},
 		{"aA", math.Log(2)},
-		{"aaA", 0.636514168294813}, // math.Log(3) - 2*math.Log(2)/3
+		{"aaA", -2*2.0/3.0*math.Log(2.0/3.0) - math.Log(1.0/3.0)/3.0},
 	}
 	for index, test := range testCases {
 		actual := CalculateEntropy(test.s, nil)
@@ -32,6 +32,35 @@ func TestStringEntropy(t *testing.T) {
 	}
 }
 
+func TestStringEntropyWithFixedProbs(t *testing.T) {
+	tolerance := 1e-6
+	a := 0.125
+	b := 0.25
+	c := 0.125
+	d := 0.5
+	probs := map[rune]float64{
+		'a': 0.125,
+		'b': 0.25,
+		'c': 0.125,
+		'd': 0.5,
+	}
+	testCases := []entropyTestCase{
+		{"", 0},
+		{"a", -a * math.Log(a)},
+		{"abc", -a*math.Log(a) - b*math.Log(b) - c*math.Log(c)},
+		{"abcdefg", -a*math.Log(a) - b*math.Log(b) - c*math.Log(c) - d*math.Log(d)},
+		{"aaaAAA", -3 * a * math.Log(a)},
+		{" a \n", -a * math.Log(a)},
+	}
+	for index, test := range testCases {
+		actual := CalculateEntropy(test.s, probs)
+		if !utils.FloatEquals(test.expected, actual, tolerance) {
+			t.Errorf("Test case %d failed (str: %s, expected: %f, actual: %f\n",
+				index+1, test.s, test.expected, actual)
+		}
+	}
+}
+
 func TestStringEntropyRatio(t *testing.T) {
 	tolerance := 1e-9
 	testCases := []entropyTestCase{
@@ -41,7 +70,7 @@ func TestStringEntropyRatio(t *testing.T) {
 		{"abcdefghij", 1},
 		{"aaa", 0},
 		{"aA", 1},
-		{"aaA", 0.5793801642856952}, // 1 - 2*math.Log(2)/(3*math.Log(3))
+		{"aaA", (-2*2.0*math.Log(2.0/3.0) - math.Log(1.0/3.0)) / (3.0 * math.Log(3))},
 	}
 	for index, test := range testCases {
 		actual := CalculateNormalisedEntropy(test.s, nil)
@@ -72,9 +101,9 @@ func TestCharacterProbabilities(t *testing.T) {
 		runeOf("d"): 1,
 		runeOf("m"): 2,
 	}
-	countsActual := CharacterCounts([]string{str, str2})
+	countsActual, _ := CharacterCounts([]string{str, str2})
 	for char, expectedCount := range countsExpected {
-		actualCount := (*countsActual)[char]
+		actualCount := countsActual[char]
 		if expectedCount != actualCount {
 			t.Errorf("Incorrect count for character '%s' (%d): expected %d, actual %d",
 				string(char), char, expectedCount, actualCount)
@@ -97,7 +126,7 @@ func TestCharacterProbabilities(t *testing.T) {
 	probsActual := CharacterProbabilities([]string{str, str2})
 
 	for char, expectedProb := range probsExpected {
-		actualProb := (*probsActual)[char]
+		actualProb := probsActual[char]
 		if !utils.FloatEquals(expectedProb, actualProb, tolerance) {
 			t.Errorf("Incorrect prob for character '%s' (%d): expected %.2f, actual %.2f",
 				string(char), char, expectedProb, actualProb)