Skip to content

Commit aee1fd5

Browse files
h0tak88rclaude
andcommitted
fix(apkauditor): kill 5 false-positive classes found auditing a real banking APK
Ran a real, in-scope banking app (io.wio.sme, Wio Bank H1 program) through the auditor and manually verified every flagged category against real decompiled smali/manifest content. 5 of ~8 categories were false positives; fixed each at the root cause: - trust_all: bare 'checkServerTrusted'/'X509TrustManager' identifiers fired on ANY implementation, including OkHttp's own legitimate internal AndroidCertificateChainCleaner (confirmed via real smali: it calls the real platform X509TrustManagerExtensions and re-throws on failure). Now requires 'TrustAllCerts' (the actual copy-paste-vulnerable class name) or a literal EMPTY checkServerTrusted method body -- the real "accepts everything" signature. - provider_query_exposed: fired on any ContentResolver.query() call with zero correlation to whether any provider is actually exported (Android forbids querying a non-exported provider at all). Now suppressed unless a genuinely exported, unprotected provider exists. - nav_deeplink_forced_navigation: fired on marker-string presence + any exported activity, without checking a navigation graph resource actually exists. The Wio app is a FlutterActivity with flutter_deeplinking_enabled=false and no res/navigation/ directory at all -- the library was only a transitive dependency, nothing was wired up to receive the deep-link extras. Now requires a real res/navigation(-qualifiers)/ path in the APK's file list. - apx_mailgun_api_key_2: the bare 'mg' alternative in '(mailgun|mg)[0-9a-z]{32}' matched almost any 34-char alphanumeric run; now requires the full 'mailgun' word. - apx_aws_access_key_id_value / apx_aws_client_id / apx_aws_api_key: used the 'gi' (case-insensitive) flag on a format AWS always issues fully uppercase, so mixed-case coincidental matches on binary-derived text (e.g. 'ASIAAAAAUMOUw0OhIZQg', which AWS would never generate) passed. Now case-sensitive. - Added looksLikeRealSecret(): a generic entropy/repetition backstop applied to all ~180 bulk-imported 'apx_*' secret patterns, rejecting matches with 4+ repeated characters or very low character diversity -- catches future binary-noise false positives in patterns not individually audited yet. Verified with real APKs (not synthetic tests): - io.wio.sme: nav_deeplink_forced_navigation/trust_all/provider_query_exposed all now 0 (were 1/6/15); mailgun/AWS noise 0 (was 64/38/38); total findings 597 -> 434. - BookBeat + BitOasis (the two confirmed true positives from the earlier nav-deeplink validation): nav_deeplink_forced_navigation still fires correctly on both -- no regression. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent fcc87b7 commit aee1fd5

1 file changed

Lines changed: 62 additions & 6 deletions

File tree

internal/api/ui/apkauditor/src/core/engine.js

Lines changed: 62 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,15 @@ const ANDROID_RULES = [
567567
},
568568
{
569569
id: 'trust_all', name: 'Trust All SSL Certificates', severity: 'issue',
570-
patterns: [/checkServerTrusted/g, /X509TrustManager/g, /TrustAllCerts/g],
570+
// Bare 'checkServerTrusted'/'X509TrustManager' identifiers fire on EVERY
571+
// legitimate implementation too -- e.g. OkHttp's own internal
572+
// AndroidCertificateChainCleaner always contains both, calls the real
573+
// platform X509TrustManagerExtensions, and re-throws on failure. Verified
574+
// against a real banking app: 100% false positive on bare identifiers.
575+
// 'TrustAllCerts' (the textbook copy-paste-vulnerable class name) and a
576+
// literal EMPTY checkServerTrusted body are the actual "accepts everything"
577+
// signatures -- require one of those instead.
578+
patterns: [/TrustAllCerts/g, /checkServerTrusted\s*\([^)]*\)(?:\s*throws\s+[\w.]+)?\s*\{\s*\}/g],
571579
description: 'Accepting all certificates makes the app vulnerable to man-in-the-middle attacks.', cwe: 'CWE-295', owasp: 'M3', masvs: 'NETWORK-4'
572580
},
573581
{
@@ -1087,13 +1095,16 @@ const ANDROID_RULES = [
10871095
patterns:[new RegExp("aws[_-]?access[_-]?key[_-]?id(=| =|:| :)", 'gi')],
10881096
description:'Detected sensitive pattern: aws_access_key_id - 1. Ensure no secrets are hardcoded.',cwe:'CWE-798',owasp:'M9',masvs:'STORAGE-14'},
10891097
{id:'apx_aws_access_key_id_value',name:"AWS Access Key ID Value",severity:'issue',
1090-
patterns:[new RegExp("(A3T[A-Z0-9]|AKIA|AGPA|AROA|AIPA|ANPA|ANVA|ASIA)[A-Z0-9]{16}", 'gi')],
1098+
// Case-SENSITIVE ('g' only): real AWS key IDs are always fully uppercase. The
1099+
// previous 'gi' flag matched mixed-case runs (e.g. "ASIAAAAAUMOUw0OhIZQg") that
1100+
// AWS never issues -- almost always a coincidental hit on binary-derived text.
1101+
patterns:[new RegExp("(A3T[A-Z0-9]|AKIA|AGPA|AROA|AIPA|ANPA|ANVA|ASIA)[A-Z0-9]{16}", 'g')],
10911102
description:'Detected sensitive pattern: AWS Access Key ID Value. Ensure no secrets are hardcoded.',cwe:'CWE-798',owasp:'M9',masvs:'STORAGE-14'},
10921103
{id:'apx_aws_api_gateway',name:"AWS API Gateway",severity:'issue',
10931104
patterns:[new RegExp("[0-9a-z]+.execute-api.[0-9a-z._-]+.amazonaws.com", 'gi')],
10941105
description:'Detected sensitive pattern: AWS API Gateway. Ensure no secrets are hardcoded.',cwe:'CWE-798',owasp:'M9',masvs:'STORAGE-14'},
10951106
{id:'apx_aws_api_key',name:"AWS API Key",severity:'issue',
1096-
patterns:[new RegExp("AKIA[0-9A-Z]{16}", 'gi')],
1107+
patterns:[new RegExp("AKIA[0-9A-Z]{16}", 'g')],
10971108
description:'Detected sensitive pattern: AWS API Key. Ensure no secrets are hardcoded.',cwe:'CWE-798',owasp:'M9',masvs:'STORAGE-14'},
10981109
{id:'apx_aws_appsync_graphql_key',name:"AWS AppSync GraphQL Key",severity:'issue',
10991110
patterns:[new RegExp("da2-[a-z0-9]{26}", 'gi')],
@@ -1102,7 +1113,8 @@ const ANDROID_RULES = [
11021113
patterns:[new RegExp("arn:aws:[a-z0-9\\-]+:[a-z]{2}-[a-z]+-[0-9]+:[0-9]+:.+", 'gi')],
11031114
description:'Detected sensitive pattern: AWS ARN. Ensure no secrets are hardcoded.',cwe:'CWE-798',owasp:'M9',masvs:'STORAGE-14'},
11041115
{id:'apx_aws_client_id',name:"AWS client ID",severity:'issue',
1105-
patterns:[new RegExp("(A3T[A-Z0-9]|AKIA|AGPA|AIDA|AROA|AIPA|ANPA|ANVA|ASIA)[A-Z0-9]{16}", 'gi')],
1116+
// Case-sensitive -- see apx_aws_access_key_id_value above.
1117+
patterns:[new RegExp("(A3T[A-Z0-9]|AKIA|AGPA|AIDA|AROA|AIPA|ANPA|ANVA|ASIA)[A-Z0-9]{16}", 'g')],
11061118
description:'Detected sensitive pattern: AWS client ID. Ensure no secrets are hardcoded.',cwe:'CWE-798',owasp:'M9',masvs:'STORAGE-14'},
11071119
{id:'apx_aws_config_accesskeyid',name:"aws_config_accesskeyid",severity:'issue',
11081120
patterns:[new RegExp("aws[_-]?config[_-]?accesskeyid(=| =|:| :)", 'gi')],
@@ -1546,7 +1558,9 @@ const ANDROID_RULES = [
15461558
patterns:[new RegExp("key-[0-9a-zA-Z]{32}", 'gi')],
15471559
description:'Detected sensitive pattern: Mailgun API Key - 1. Ensure no secrets are hardcoded.',cwe:'CWE-798',owasp:'M9',masvs:'STORAGE-14'},
15481560
{id:'apx_mailgun_api_key_2',name:"Mailgun API key - 2",severity:'issue',
1549-
patterns:[new RegExp("(mailgun|mg)[0-9a-z]{32}", 'gi')],
1561+
// 'mg' alone (without the full 'mailgun' word) as a 2-char prefix matched almost
1562+
// any 34-char alphanumeric run in binary-derived string data -- dropped it.
1563+
patterns:[new RegExp("mailgun[0-9a-z]{32}", 'gi')],
15501564
description:'Detected sensitive pattern: Mailgun API key - 2. Ensure no secrets are hardcoded.',cwe:'CWE-798',owasp:'M9',masvs:'STORAGE-14'},
15511565
{id:'apx_mailgun_apikey',name:"mailgun_apikey",severity:'issue',
15521566
patterns:[new RegExp("mailgun[_-]?apikey(=| =|:| :)", 'gi')],
@@ -1824,6 +1838,23 @@ function extractManifestInfo(R) {
18241838
}
18251839
}
18261840

1841+
// Sanity backstop for the bulk-imported 'apx_*' secret-pattern rules (~180 loose
1842+
// keyword/format regexes). Large real APKs contain a lot of binary-derived text
1843+
// (compressed assets, native blobs) that coincidentally satisfies a short
1844+
// fixed-prefix + alnum-run pattern. Real secrets are high-entropy; coincidental
1845+
// matches on binary noise tend to have repeated characters or low character
1846+
// diversity. This doesn't validate a secret is real -- it only filters out the
1847+
// most obviously-not-random matches, which measurably cut false positives on a
1848+
// real banking-app APK (e.g. "ASIAAAAAUMOUw0OhIZQg") without touching well-formed
1849+
// keys we could verify independently (e.g. a real "AIzaSy..." Google API key).
1850+
function looksLikeRealSecret(s) {
1851+
if (!s || s.length < 10) return true; // too short to judge meaningfully either way
1852+
if (/(.)\1{3,}/.test(s)) return false; // 4+ identical chars in a row
1853+
const uniq = new Set(s.toLowerCase()).size;
1854+
if (s.length >= 12 && uniq / s.length < 0.35) return false; // too repetitive for random data
1855+
return true;
1856+
}
1857+
18271858
function analyzeContent(content, filePath, rules) {
18281859
const findings = [];
18291860
const safe = content.length > 300000 ? content.slice(0, 300000) : content;
@@ -1833,6 +1864,10 @@ function analyzeContent(content, filePath, rules) {
18331864
pat.lastIndex = 0;
18341865
let m, count = 0;
18351866
while ((m = pat.exec(safe)) !== null && count++ < 20) {
1867+
if (rule.id.startsWith('apx_') && !looksLikeRealSecret(m[0])) {
1868+
if (m.index === pat.lastIndex) pat.lastIndex++;
1869+
continue;
1870+
}
18361871
const ln = (safe.substring(0, m.index).match(/\n/g) || []).length + 1;
18371872
findings.push({
18381873
ruleId: rule.id, ruleName: rule.name, severity: rule.severity,
@@ -3140,7 +3175,15 @@ async function analyzeAPK(arrayBuffer, fileMeta, opts) {
31403175
// primitive (CWE-284) — a malicious co-installed app can pass
31413176
// android-support-nav:controller:deepLinkIds/deepLinkArgs/deepLinkExtras to
31423177
// force navigation to arbitrary destinations, bypassing session/login gating.
3143-
if (R.findings.some(f => f.ruleId === 'nav_deeplink_ids')) {
3178+
//
3179+
// A real, in-scope banking app (io.wio.sme) false-positived on this: the marker
3180+
// string was present only because a transitively-bundled library depends on
3181+
// androidx.navigation, but the app itself (a FlutterActivity with
3182+
// flutter_deeplinking_enabled=false) had no res/navigation/ graph at all --
3183+
// there was nothing for handleDeepLink() to actually navigate to. Require an
3184+
// actual navigation-graph resource in the APK before escalating.
3185+
const hasNavGraphResource = (R.files || []).some(f => /^res\/navigation(-[^/]+)?\//i.test(f));
3186+
if (R.findings.some(f => f.ruleId === 'nav_deeplink_ids') && hasNavGraphResource) {
31443187
const exposedActivities = (R.components && R.components.activities || []).filter(a => a.exported && !a.permission);
31453188
if (exposedActivities.length > 0) {
31463189
const names = exposedActivities.map(a => a.name).slice(0, 5).join(', ') + (exposedActivities.length > 5 ? `, +${exposedActivities.length - 5} more` : '');
@@ -3156,6 +3199,19 @@ async function analyzeAPK(arrayBuffer, fileMeta, opts) {
31563199
}
31573200
}
31583201

3202+
// provider_query_exposed fires on any ContentResolver.query() call in decompiled
3203+
// code with zero awareness of whether any provider is actually exported --
3204+
// Android forbids other apps from querying a non-exported provider at all, so
3205+
// without a genuinely exported+unprotected provider this can never be
3206+
// exploited by another app. Drop it entirely when none exists (the common case:
3207+
// apps routinely query their own local/non-exported providers as normal code).
3208+
if (R.findings.some(f => f.ruleId === 'provider_query_exposed')) {
3209+
const exposedProviders = (R.components && R.components.providers || []).filter(p => p.exported && !p.permission && !p.readPermission);
3210+
if (exposedProviders.length === 0) {
3211+
R.findings = R.findings.filter(f => f.ruleId !== 'provider_query_exposed');
3212+
}
3213+
}
3214+
31593215
onProgress(90, 'Detecting trackers');
31603216
R.trackers = detectTrackers(allDexStrings, R.files);
31613217

0 commit comments

Comments
 (0)