fix: required-reason cross-check reads symbols, not strings - #20
Open
adamXbot wants to merge 1 commit into
Open
Conversation
PrivacyManifestReader.crossCheck now matches the manifest's declared NSPrivacyAccessedAPITypes against the binary's undefined-external symbols (MachOInspector.importedSymbols), using a full RequiredReasonAPIs vocabulary table with exact nlist spellings for all five documented categories. MachOInspector.importedSymbols now parses LC_SYMTAB in every arch slice of a universal binary and returns the de-duplicated union. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause
The privacy-manifest cross-check compared the manifest's
NSPrivacyAccessedAPITypesagainstBinaryStringScanner.foundFrameworkSymbols— ASCII strings scraped from the binary — using an 11-entry lookup table. Three separate defects:stat,fstat,lstat,fstatat,getattrlist,statfs,mach_absolute_timeand friends are the core of Apple's documented required-reason symbol lists; most were missing, so binaries using them were reported as clean.CMPedometer(a CoreMotion class, not a required-reason API at all) to the UserDefaults category, annotated "close enough for cross-check". Any app embedding the stringCMPedometerwas flagged as using UserDefaults.MachOInspector.importedSymbolsparsedLC_SYMTABof only the first slice of a fat binary, so evidence present only in the other slice (including x86_64-specific spellings like_stat$INODE64) was invisible.String matching itself is also the wrong mechanism: the presence of an ASCII string like
creationDatein a binary is not evidence the API is called, and its absence is not evidence it isn't.What changed
MachOInspector.importedSymbolsnow enumerates every arch slice and returns the de-duplicated union of undefined-external symbols across all slices.Analysis/RequiredReasonAPIs.swift: Apple's required-reason vocabulary as data — all fiveNSPrivacyAccessedAPICategory*values, their approved reason codes with restrictions, and their symbol lists with exact nlist spellings (_stat,_stat$INODE64,_OBJC_CLASS_$_NSUserDefaults, …), keyed to Apple's documentation. The old 11-entry table and itsCMPedometerentry are deleted.PrivacyManifestReader.crossChecknow takesimportedSymbols: Set<String>and matches exactly on nlist spellings — never substring-based. A symbol in two categories (thegetattrlistfamily is in both FileTimestamp and DiskSpace) counts as evidence for each.StaticAnalyzer,StaticAnalysisView) feedMachOInspector.importedSymbols(of:)instead of a string-scan result.RequiredReasonAPIs.swiftis also registered inproject.pbxproj.What findings mean now
__LINKEDITis not encrypted). Expect more of these findings, and expect them to name real symbols.objc_msgSend(e.g.ProcessInfo.systemUptime→_OBJC_CLASS_$_NSProcessInfo).importedSymbolsnow unions all slices,BinaryCapabilityAnalyzer(which consumes the same call) may report more capabilities on universal binaries than a first-slice read did — correct behaviour, and it may shift risk scores. The 9ManifestTrackingMismatchTestswere watched for fallout: all 9 pass unchanged.Cache compatibility
Conservative path taken: no Codable shape change, no cache version bump needed.
StaticReportpersistsPrivacyManifest(unchanged, byte-compatible encoding) and derivedFindingstrings;PrivacyManifestCrossCheckitself is computed on demand and never persisted. Cached reports remain loadable; their stored findings reflect the old detection until re-analysis (the cache key already rotates on auditor version and binary mtime).Flagged, deliberately not done here
User-facing copy and the knowledge-base article for
privacy-manifestwere left untouched (the existing copy says "symbol references", which is accurate for the new mechanism, but a fuller KB pass — e.g. explaining nlist evidence and reason codes — is an open decision for the maintainer).Verification
main):swift test— 325 executed, 4 skipped, 0 failuresswift test— 325 executed, 4 skipped, 0 failures (identical; no test-by-test delta)swift test --filter ManifestTrackingMismatchTests— 9 executed, 0 failuresxcodebuild -project privacycommand/privacycommand.xcodeproj -scheme privacycommand -configuration Debug -destination 'platform=macOS' build— BUILD SUCCEEDED🤖 Generated with Claude Code