Skip to content

fix(playscan): detect per-library 64-bit gaps #28

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
snowyukitty wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(playscan): detect per-library 64-bit gaps #28

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
73 changes: 54 additions & 19 deletions internal/playscan/archive.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,32 +159,67 @@ func check64BitParity(libs []nativeLib) []Finding {
return nil
}

present := make(map[string]bool)
libsByABI := make(map[string]map[string]bool)
for _, lib := range libs {
present[lib.ABI] = true
if libsByABI[lib.ABI] == nil {
libsByABI[lib.ABI] = make(map[string]bool)
}
libsByABI[lib.ABI][path.Base(lib.Path)] = true
}

var missing []string
var missingABIs, partialGaps []string
for abi32, abi64 := range abi64For {
if present[abi32] && !present[abi64] {
missing = append(missing, fmt.Sprintf("%s is present with no %s", abi32, abi64))
libs32 := libsByABI[abi32]
if len(libs32) == 0 {
continue
}
libs64 := libsByABI[abi64]
if len(libs64) == 0 {
missingABIs = append(missingABIs, fmt.Sprintf("%s is present with no %s", abi32, abi64))
continue
}

var missingLibs []string
for name := range libs32 {
if !libs64[name] {
missingLibs = append(missingLibs, name)
}
}
if len(missingLibs) > 0 {
sort.Strings(missingLibs)
partialGaps = append(partialGaps, fmt.Sprintf(
"%s -> %s: %s", abi32, abi64, strings.Join(missingLibs, ", "),
))
}
}
if len(missing) == 0 {
return nil
sort.Strings(missingABIs)
sort.Strings(partialGaps)

var findings []Finding
if len(missingABIs) > 0 {
findings = append(findings, Finding{
Severity: sevCritical,
Policy: "64-bit requirement",
Title: "32-bit native code ships without its 64-bit counterpart",
Detail: "Google Play requires every app with native code to provide a 64-bit library for each 32-bit ABI it supports. " +
strings.Join(missingABIs, "; ") + ". Play rejects a bundle that carries only 32-bit native code for an ABI.",
Fix: "Add the 64-bit ABIs to your NDK build (abiFilters or ndk.abiFilters), rebuild, and confirm every dependency ships 64-bit libraries too. " +
"Dropping the 32-bit ABI entirely also satisfies the requirement.",
Doc: doc64Bit,
})
}
sort.Strings(missing)

return []Finding{{
Severity: sevCritical,
Policy: "64-bit requirement",
Title: "32-bit native code ships without its 64-bit counterpart",
Detail: "Google Play requires every app with native code to provide a 64-bit library for each 32-bit ABI it supports. " +
strings.Join(missing, "; ") + ". Play rejects a bundle that carries only 32-bit native code for an ABI.",
Fix: "Add the 64-bit ABIs to your NDK build (abiFilters or ndk.abiFilters), rebuild, and confirm every dependency ships 64-bit libraries too. " +
"Dropping the 32-bit ABI entirely also satisfies the requirement.",
Doc: doc64Bit,
}}
if len(partialGaps) > 0 {
findings = append(findings, Finding{
Severity: sevHigh,
Policy: "64-bit requirement",
Title: "32-bit native libraries are missing 64-bit builds",
Detail: "These 32-bit libraries have no matching build in the corresponding 64-bit ABI: " +
strings.Join(partialGaps, "; ") + ". Calling System.loadLibrary for one of them on a 64-bit-only device fails with UnsatisfiedLinkError.",
Fix: "Build each named library for the corresponding 64-bit ABI, or remove its 32-bit build if that ABI is no longer supported.",
Doc: doc64Bit,
})
}
return findings
}

// classifyArchive determines the format and locates the manifest entry.
Expand Down
95 changes: 92 additions & 3 deletions internal/playscan/archive_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,10 @@ func writeChunkTo(w *bytes.Buffer, chunkType uint16, payload []byte) {
// forms that matter: a string permission name, a boolean, an integer SDK
// level, and a foregroundServiceType bitmask.
func buildTestManifest() []byte {
return buildTestManifestWithSplit("")
}

func buildTestManifestWithSplit(split string) []byte {
b := &axmlBuilder{}
nameAttr := b.attrRef(0x01010003)
debuggableAttr := b.attrRef(0x0101000f)
Expand All @@ -163,9 +167,17 @@ func buildTestManifest() []byte {
launcherIdx := b.str("android.intent.category.LAUNCHER")
packageAttr := b.str("package")

b.startTag("manifest", []buildAttr{
manifestAttrs := []buildAttr{
{nameIdx: packageAttr, dataType: typeString, data: pkgIdx, rawIdx: pkgIdx},
})
}
if split != "" {
splitAttr := b.str("split")
splitIdx := b.str(split)
manifestAttrs = append(manifestAttrs, buildAttr{
nameIdx: splitAttr, dataType: typeString, data: splitIdx, rawIdx: splitIdx,
})
}
b.startTag("manifest", manifestAttrs)
b.startTag("uses-permission", []buildAttr{
{nameIdx: nameAttr, dataType: typeString, data: permIdx, rawIdx: permIdx},
})
Expand Down Expand Up @@ -309,6 +321,11 @@ func buildTestELF(align uint64, relro bool) []byte {
// buildTestAPK writes an APK containing a compiled manifest and the given
// native libraries.
func buildTestAPK(t *testing.T, libs map[string][]byte) string {
t.Helper()
return buildTestAPKWithManifest(t, buildTestManifest(), libs)
}

func buildTestAPKWithManifest(t *testing.T, manifest []byte, libs map[string][]byte) string {
t.Helper()
path := filepath.Join(t.TempDir(), "app.apk")
f, err := os.Create(path)
Expand All @@ -322,7 +339,7 @@ func buildTestAPK(t *testing.T, libs map[string][]byte) string {
if err != nil {
t.Fatalf("zip create: %v", err)
}
if _, err := mw.Write(buildTestManifest()); err != nil {
if _, err := mw.Write(manifest); err != nil {
t.Fatalf("write manifest: %v", err)
}
for name, data := range libs {
Expand Down Expand Up @@ -869,6 +886,54 @@ func TestScanArchive64BitParity(t *testing.T) {
}
})

t.Run("partial library gap is named", func(t *testing.T) {
apk := buildTestAPK(t, map[string][]byte{
"lib/armeabi-v7a/libzeta.so": buildTestELF(16384, true),
"lib/armeabi-v7a/libfoo.so": buildTestELF(16384, true),
"lib/armeabi-v7a/libbar.so": buildTestELF(16384, true),
"lib/arm64-v8a/libfoo.so": buildTestELF(16384, true),
})
res, err := ScanArchive(apk)
if err != nil {
t.Fatalf("ScanArchive: %v", err)
}
f := findByPolicy(res.Findings, "64-bit requirement")
if f == nil {
t.Fatal("a 32-bit library without a matching 64-bit build was not flagged")
}
if f.Severity != sevHigh {
t.Errorf("severity = %v, want HIGH", f.Severity)
}
if !strings.Contains(f.Detail, "libbar.so, libzeta.so") {
t.Errorf("detail should name missing libraries in sorted order: %s", f.Detail)
}
if strings.Contains(f.Detail, "libfoo.so") {
t.Errorf("detail should not name a library that has a 64-bit build: %s", f.Detail)
}
})

t.Run("whole and partial gaps are reported separately", func(t *testing.T) {
apk := buildTestAPK(t, map[string][]byte{
"lib/armeabi-v7a/libfoo.so": buildTestELF(16384, true),
"lib/armeabi-v7a/libbar.so": buildTestELF(16384, true),
"lib/arm64-v8a/libfoo.so": buildTestELF(16384, true),
"lib/x86/libnative.so": buildTestELF(16384, true),
})
res, err := ScanArchive(apk)
if err != nil {
t.Fatalf("ScanArchive: %v", err)
}
if got := countByPolicy(res.Findings, "64-bit requirement"); got != 2 {
t.Fatalf("64-bit findings = %d, want one CRITICAL and one HIGH", got)
}
if f := findByPolicy(res.Findings, "64-bit requirement"); f.Severity != sevCritical {
t.Errorf("first 64-bit finding severity = %v, want CRITICAL", f.Severity)
}
if f := findByTitle(res.Findings, "32-bit native libraries are missing 64-bit builds"); f == nil || f.Severity != sevHigh {
t.Errorf("partial gap finding = %+v, want HIGH", f)
}
})

t.Run("both ABIs present is clean", func(t *testing.T) {
apk := buildTestAPK(t, map[string][]byte{
"lib/armeabi-v7a/libnative.so": buildTestELF(16384, true),
Expand Down Expand Up @@ -896,6 +961,30 @@ func TestScanArchive64BitParity(t *testing.T) {
}
})

t.Run("no native code is clean", func(t *testing.T) {
apk := buildTestAPK(t, nil)
res, err := ScanArchive(apk)
if err != nil {
t.Fatalf("ScanArchive: %v", err)
}
if f := findByPolicy(res.Findings, "64-bit requirement"); f != nil {
t.Errorf("an app without native code should not be flagged: %s", f.Title)
}
})

t.Run("config split is exempt", func(t *testing.T) {
apk := buildTestAPKWithManifest(t, buildTestManifestWithSplit("config.armeabi_v7a"), map[string][]byte{
"lib/armeabi-v7a/libnative.so": buildTestELF(16384, true),
})
res, err := ScanArchive(apk)
if err != nil {
t.Fatalf("ScanArchive: %v", err)
}
if f := findByPolicy(res.Findings, "64-bit requirement"); f != nil {
t.Errorf("a config split should not be checked in isolation: %s", f.Title)
}
})

t.Run("x86 without x86_64 is flagged", func(t *testing.T) {
apk := buildTestAPK(t, map[string][]byte{
"lib/x86/libnative.so": buildTestELF(16384, true),
Expand Down