Skip to content

fix: required-reason cross-check reads symbols, not strings #20

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
adamXbot wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: required-reason cross-check reads symbols, not strings #20

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion privacycommand/Sources/privacycommand/Views/StaticAnalysisView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ struct StaticAnalysisView: View {
crossCheck: report.privacyManifest.map {
PrivacyManifestReader.crossCheck(
manifest: $0,
scan: BinaryStringScanner.scan(executable: report.bundle.executableURL))
importedSymbols: Set(MachOInspector.importedSymbols(
of: report.bundle.executableURL)))
})
.id(SectionAnchor.privacyManifest.rawValue)
SandboxContainerView(info: sandboxContainer)
Expand Down
68 changes: 20 additions & 48 deletions privacycommand/Sources/privacycommandCore/Analysis/MachOInspector.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,9 @@ public enum MachOInspector {
buildPlatform: nil, sliceCount: 0)
}

/// Best-effort parse of the first thin slice (or first arch of a fat
/// binary) for the load-command details we care about. Returns
/// `.empty` for unrecognised formats rather than throwing — this is a
/// soft analysis, not a load-bearing parser.
/// Best-effort parse of every arch slice for the load-command details we
/// care about. Returns `.empty` for unrecognised formats rather than
/// throwing — this is a soft analysis, not a load-bearing parser.
public static func loadCommands(of url: URL) -> LoadCommandsSummary {
guard let data = try? Data(contentsOf: url, options: [.mappedIfSafe]),
data.count >= 32 else { return .empty }
Expand Down Expand Up @@ -78,8 +77,7 @@ public enum MachOInspector {
}

/// Enumerate the (offset, is64) of every Mach-O slice -- one entry for a
/// thin file, every fat arch for a universal binary. Generalises
/// `firstThinSlice` (which the symbol/arch readers still use).
/// thin file, every fat arch for a universal binary.
private static func enumerateSlices(in data: Data) -> [(Int, Bool)] {
let magic = data.withUnsafeBytes { $0.loadUnaligned(as: UInt32.self) }
if magic == mh_magic || magic == mh_cigam { return [(0, false)] }
Expand Down Expand Up @@ -132,16 +130,27 @@ public enum MachOInspector {
/// disassembly, and readable even on encrypted App Store binaries (the
/// `__LINKEDIT` symbol table is not encrypted).
///
/// Parses the first thin slice's `LC_SYMTAB` directly — no `nm`/`objdump`
/// dependency, so it works even without Xcode Command Line Tools.
/// Parses `LC_SYMTAB` in EVERY arch slice directly — no `nm`/`objdump`
/// dependency, so it works even without Xcode Command Line Tools. Slices
/// of a universal binary can import different symbols (and x86_64 slices
/// spell some libc imports differently, e.g. `_stat$INODE64`), so the
/// result is the de-duplicated union across all slices: a symbol imported
/// by only one slice is still reported.
/// Leading underscores are preserved (Mach-O convention). Returns at most
/// `limit` symbols. Returns `[]` (never throws) for unparseable inputs.
public static func importedSymbols(of url: URL, limit: Int = 8000) -> [String] {
guard let data = try? Data(contentsOf: url, options: [.mappedIfSafe]),
data.count >= 32 else { return [] }
let (sliceOffset, is64) = firstThinSlice(in: data)
guard let sliceOffset else { return [] }
return parseImportedSymbols(in: data, at: sliceOffset, is64: is64, limit: limit)
var out: [String] = []
var seen = Set<String>()
for (sliceOffset, is64) in enumerateSlices(in: data) {
guard out.count < limit else { break }
for name in parseImportedSymbols(in: data, at: sliceOffset, is64: is64, limit: limit) {
guard out.count < limit else { break }
if seen.insert(name).inserted { out.append(name) }
}
}
return out
}

public static func architectures(of url: URL) throws -> [String] {
Expand Down Expand Up @@ -209,8 +218,6 @@ public enum MachOInspector {

// MARK: - Load command parsing

/// Locate the start of the first thin Mach-O slice. For a thin binary
/// the offset is 0; for a fat binary we read the first arch's offset.
/// Decode a Mach-O nibble-packed version (X.Y.Z in bits xxxx.yy.zz).
static func decodeVersion(_ v: UInt32) -> String {
let x = (v >> 16) & 0xFFFF, y = (v >> 8) & 0xFF, z = v & 0xFF
Expand All @@ -234,41 +241,6 @@ public enum MachOInspector {
}
}

private static func firstThinSlice(in data: Data) -> (Int?, Bool) {
let magic = data.withUnsafeBytes { $0.loadUnaligned(as: UInt32.self) }
if magic == mh_magic || magic == mh_cigam { return (0, false) }
if magic == mh_magic_64 || magic == mh_cigam_64 { return (0, true) }
let isFat = magic == fatMagic || magic == fatMagicSwapped
|| magic == fat64Magic || magic == fat64MagicSwapped
guard isFat, data.count >= 16 else { return (nil, false) }
let swapped = magic == fatMagicSwapped || magic == fat64MagicSwapped
let is64 = magic == fat64Magic || magic == fat64MagicSwapped
// First arch starts at 8; offset field is at +8 (32-bit) or +8 (64-bit).
// 32-bit arch struct: cputype(4) cpusubtype(4) offset(4) size(4) align(4)
// 64-bit arch struct: cputype(4) cpusubtype(4) offset(8) size(8) align(4) reserved(4)
let offsetField = 8 + 8
let offset: UInt64
if is64 {
var raw: UInt64 = 0
data.withUnsafeBytes {
raw = $0.baseAddress!.advanced(by: offsetField).loadUnaligned(as: UInt64.self)
}
offset = swapped ? raw.byteSwapped : raw
} else {
var raw: UInt32 = 0
data.withUnsafeBytes {
raw = $0.baseAddress!.advanced(by: offsetField).loadUnaligned(as: UInt32.self)
}
offset = UInt64(swapped ? raw.byteSwapped : raw)
}
guard offset < UInt64(data.count) else { return (nil, false) }
let sliceMagic = data.withUnsafeBytes {
$0.baseAddress!.advanced(by: Int(offset)).loadUnaligned(as: UInt32.self)
}
let sliceIs64 = sliceMagic == mh_magic_64 || sliceMagic == mh_cigam_64
return (Int(offset), sliceIs64)
}

private static func parseThinLoadCommands(in data: Data, at sliceOffset: Int, is64: Bool) -> LoadCommandsSummary {
// mach_header(_64): magic(4) cputype(4) cpusubtype(4) filetype(4)
// ncmds(4) sizeofcmds(4) flags(4) [reserved(4) for 64]
Expand Down
50 changes: 22 additions & 28 deletions privacycommand/Sources/privacycommandCore/Analysis/PrivacyManifestReader.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,37 +102,31 @@ public enum PrivacyManifestReader {

// MARK: - Cross-check

/// Compare the manifest's `accessedAPITypes` against the binary scan's
/// observed symbol references. Returns mismatches in both directions.
/// Compare the manifest's `accessedAPITypes` against the binary's
/// undefined-external symbols (`MachOInspector.importedSymbols(of:)`),
/// using the `RequiredReasonAPIs` vocabulary. Returns mismatches in both
/// directions.
///
/// Matching is exact on nlist spellings — `_stat`, `_stat$INODE64`,
/// `_OBJC_CLASS_$_NSUserDefaults` — never substring-based. A symbol that
/// belongs to more than one category (the `getattrlist` family sits in
/// both FileTimestamp and DiskSpace) counts as evidence for each.
///
/// Categories the manifest declares that Apple doesn't document (they
/// collapse to `.other`) are excluded from `declaredButUnused`: no symbol
/// evidence can exist for them, and flagging the unknown value itself is
/// a validity check, not a cross-check.
public static func crossCheck(manifest: PrivacyManifest,
scan: BinaryStringScanner.Result) -> PrivacyManifestCrossCheck {
let symbolToCategory: [String: PrivacyManifest.AccessedAPI.Category] = [
// File timestamps
"NSFileSystemNumber": .fileTimestamp,
"creationDate": .fileTimestamp,
"fileModificationDate": .fileTimestamp,
// Disk space
"volumeAvailableCapacityKey": .diskSpace,
"systemFreeSize": .diskSpace,
"NSURLVolumeAvailableCapacityKey": .diskSpace,
// System boot time
"kern.boottime": .systemBootTime,
"mach_absolute_time": .systemBootTime,
// User defaults
"NSUserDefaults": .userDefaults,
// Active keyboards
"TIInputSource": .activeKeyboards,
// CoreMotion / pedometer
"CMPedometer": .userDefaults, // close enough for cross-check
]

var symbolEvidenceByCategory: [PrivacyManifest.AccessedAPI.Category: [String]] = [:]
for (symbol, cat) in symbolToCategory where scan.foundFrameworkSymbols.contains(symbol) {
symbolEvidenceByCategory[cat, default: []].append(symbol)
importedSymbols: Set<String>) -> PrivacyManifestCrossCheck {
var evidenceByCategory: [PrivacyManifest.AccessedAPI.Category: Set<String>] = [:]
for symbol in importedSymbols {
for category in RequiredReasonAPIs.categories(forNlistSpelling: symbol) {
evidenceByCategory[category, default: []].insert(symbol)
}
}

let declaredCategories = Set(manifest.accessedAPITypes.map(\.category))
let observedCategories = Set(symbolEvidenceByCategory.keys)
let observedCategories = Set(evidenceByCategory.keys)

let declaredButUnused = declaredCategories.subtracting(observedCategories)
.filter { $0 != .other }
Expand All @@ -142,7 +136,7 @@ public enum PrivacyManifestReader {
declaredButUnused: declaredButUnused.sorted(by: { $0.rawValue < $1.rawValue }),
usedButUndeclared: usedButUndeclared.sorted(by: { $0.rawValue < $1.rawValue })
.map { PrivacyManifestCrossCheck.Mismatch(
category: $0, evidence: symbolEvidenceByCategory[$0] ?? []) }
category: $0, evidence: (evidenceByCategory[$0] ?? []).sorted()) }
)
}

Expand Down
Loading