Repository navigation
fix(android): report split-required manifests as a JNI library unknown #9769
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.
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
base: main
Are you sure you want to change the base?
Changes from 7 commits
8b79781
efecd1c
35c9cd5
9f5daf4
4937c1d
7000218
63dc1bd
a3db6d9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,334 @@ | ||
| import { | ||
| BinaryFormatFailure, | ||
| BinaryReader, | ||
| } from "../native/metadata/BinaryContainer.js"; | ||
| import { err, ok, type Result } from "../domain/result.js"; | ||
|
|
||
| /** Split declarations the binary manifest owns; a base APK's libraries may live in these splits. */ | ||
| export interface AndroidBinaryManifestSplitFacts { | ||
| readonly required_split_types: readonly string[] | null; | ||
| readonly split_name: string | null; | ||
| readonly is_split_required: boolean | null; | ||
| } | ||
|
|
||
| export type AndroidBinaryManifestSplitFailure = { | ||
| readonly kind: "unsupported" | "malformed" | "capacity"; | ||
| readonly message: string; | ||
| }; | ||
|
|
||
| const RES_XML_TYPE = 0x0003; | ||
| const RES_STRING_POOL_TYPE = 0x0001; | ||
| const RES_XML_RESOURCE_MAP_TYPE = 0x0180; | ||
| const RES_XML_START_ELEMENT_TYPE = 0x0102; | ||
| const ANDROID_NS = "http://schemas.android.com/apk/res/android"; | ||
| const MAXIMUM_STRING_BYTES = 1024 * 1024; | ||
| const MAXIMUM_STRINGS = 100_000; | ||
| const UTF8_FLAG = 0x100; | ||
| const ATTRIBUTE_ROWS = 20; | ||
| const RES_XML_NODE_HEADER = 16; | ||
| /** Framework resource IDs disambiguate the attributes from name spelling and prefix binding. */ | ||
| const REQUIRED_SPLIT_TYPES_ID = 0x0101064c; | ||
| const IS_SPLIT_REQUIRED_ID = 0x0101064d; | ||
| const SPLIT_NAME_ID = 0x01010549; | ||
| const TYPE_STRING = 0x03; | ||
| const TYPE_INT_BOOLEAN = 0x12; | ||
|
|
||
| const decode = ( | ||
| bytes: Buffer, | ||
| encoding: "utf-8" | "utf-16le", | ||
| ): string | null => { | ||
| try { | ||
| return new TextDecoder(encoding, { fatal: true }).decode(bytes); | ||
| } catch { | ||
| return null; | ||
| } | ||
| }; | ||
|
|
||
| /** One or two UTF-8 bytes: the high bit selects the two-byte form. */ | ||
| const utf8Length = ( | ||
| reader: BinaryReader, | ||
| at: number, | ||
| end: number, | ||
| ): { readonly value: number; readonly consumed: number } | null => { | ||
| if (at >= end) return null; | ||
| const first = reader.bytes[at] ?? 0; | ||
| if ((first & 0x80) === 0) return { value: first, consumed: 1 }; | ||
| if (at + 1 >= end) return null; | ||
| return { | ||
| value: ((first & 0x7f) << 8) | (reader.bytes[at + 1] ?? 0), | ||
| consumed: 2, | ||
| }; | ||
| }; | ||
|
|
||
| const readStringPool = ( | ||
| reader: BinaryReader, | ||
| start: number, | ||
| headerSize: number, | ||
| chunkSize: number, | ||
| ): Result<readonly (string | null)[], AndroidBinaryManifestSplitFailure> => { | ||
| const count = reader.u32(start + 8); | ||
| const flags = reader.u32(start + 16); | ||
| const stringsStart = reader.u32(start + 20); | ||
| const bad = (message: string) => err({ kind: "malformed" as const, message }); | ||
| if (count > MAXIMUM_STRINGS) | ||
| return err({ | ||
| kind: "capacity", | ||
| message: `The manifest string pool declares more than ${String(MAXIMUM_STRINGS)} entries.`, | ||
| }); | ||
| if (headerSize < 28 || chunkSize < headerSize) | ||
| return bad("The manifest string pool header is truncated."); | ||
| if (count > (chunkSize - headerSize) / 4) | ||
| return bad("The manifest string pool offsets leave the chunk."); | ||
| if (stringsStart < headerSize || stringsStart > chunkSize) | ||
| return bad("The manifest string pool data starts outside its chunk."); | ||
| const dataStart = start + stringsStart; | ||
| const dataEnd = start + chunkSize; | ||
| const utf8 = (flags & UTF8_FLAG) !== 0; | ||
| const strings: (string | null)[] = []; | ||
| let budget = MAXIMUM_STRING_BYTES; | ||
| for (let index = 0; index < count; index++) { | ||
| const relative = reader.u32(start + headerSize + index * 4); | ||
| let at = dataStart + relative; | ||
| const fail = (): void => { | ||
| strings.push(null); | ||
| }; | ||
| if (at < dataStart || at >= dataEnd) { | ||
| fail(); | ||
| continue; | ||
| } | ||
| let dataLength: number; | ||
| let skip: number; | ||
| if (!utf8) { | ||
| if (at + 2 > dataEnd) { | ||
| fail(); | ||
| continue; | ||
| } | ||
| dataLength = reader.u16(at) * 2; | ||
| skip = 2; | ||
| } else { | ||
| const units = utf8Length(reader, at, dataEnd); | ||
| const byteUnits = | ||
| units === null | ||
| ? null | ||
| : utf8Length(reader, at + units.consumed, dataEnd); | ||
| if (units === null || byteUnits === null) { | ||
| fail(); | ||
| continue; | ||
| } | ||
| dataLength = byteUnits.value; | ||
| skip = units.consumed + byteUnits.consumed; | ||
| } | ||
| const dataAt = at + skip; | ||
| if (dataLength > budget || dataAt + dataLength > dataEnd) { | ||
| fail(); | ||
| continue; | ||
| } | ||
| strings.push( | ||
| decode( | ||
| reader.range(dataAt, dataLength, "manifest string"), | ||
| utf8 ? "utf-8" : "utf-16le", | ||
| ), | ||
| ); | ||
| budget -= dataLength; | ||
| } | ||
| return ok(strings); | ||
| }; | ||
|
|
||
| const splitTypes = (value: string): readonly string[] => | ||
| value | ||
| .split(",") | ||
| .map((part) => part.trim()) | ||
| .filter((part) => part.length > 0); | ||
|
|
||
| interface StartElement { | ||
| readonly name: string | null; | ||
| readonly attributes: readonly { | ||
| readonly namespace: string | null; | ||
| readonly name: string | null; | ||
| readonly resourceId: number | undefined; | ||
| readonly raw: string | null; | ||
| readonly dataType: number; | ||
| readonly data: number; | ||
| }[]; | ||
| } | ||
|
|
||
| const readStartElement = ( | ||
| reader: BinaryReader, | ||
| strings: readonly (string | null)[], | ||
| resourceIds: readonly number[], | ||
| start: number, | ||
| headerSize: number, | ||
| chunkSize: number, | ||
| ): StartElement | null => { | ||
| // A start element begins with the common 16-byte ResXMLNode header; a chunk | ||
| // that advertises less than that places the name and attribute base outside | ||
| // its own header, so its declarations can only be read as absent. | ||
| if (headerSize < RES_XML_NODE_HEADER) return null; | ||
| const attributeStart = reader.u16(start + 24); | ||
| const attributeSize = reader.u16(start + 26); | ||
| const attributeCount = reader.u16(start + 28); | ||
| if (attributeSize < ATTRIBUTE_ROWS) return null; | ||
| const name = strings[reader.u32(start + 20)] ?? null; | ||
| const table = start + reader.u16(start + 2) + attributeStart; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a start-element chunk has AGENTS.md reference: AGENTS.md:L21-L21 Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in |
||
| const attributes: StartElement["attributes"][number][] = []; | ||
| for (let index = 0; index < attributeCount; index++) { | ||
| const at = table + index * attributeSize; | ||
| if (at < table || at + ATTRIBUTE_ROWS > start + chunkSize) return null; | ||
| const nameIndex = reader.u32(at + 4); | ||
| const rawValueIndex = reader.u32(at + 8); | ||
| attributes.push({ | ||
| namespace: strings[reader.u32(at)] ?? null, | ||
| name: strings[nameIndex] ?? null, | ||
| resourceId: resourceIds[nameIndex], | ||
| raw: | ||
| rawValueIndex === 0xffffffff ? null : (strings[rawValueIndex] ?? null), | ||
| dataType: reader.bytes[at + 15] ?? 0, | ||
| data: reader.u32(at + 16), | ||
| }); | ||
| } | ||
| return { name, attributes }; | ||
| }; | ||
|
|
||
| /** Read only the split-declaration attributes from an admitted binary AndroidManifest.xml. */ | ||
| export const readAndroidBinaryManifestSplitFacts = ( | ||
| bytes: Buffer, | ||
| ): Result< | ||
| AndroidBinaryManifestSplitFacts, | ||
| AndroidBinaryManifestSplitFailure | ||
| > => { | ||
| try { | ||
| if ( | ||
| bytes.length < 8 || | ||
| bytes.readUInt16LE(0) !== RES_XML_TYPE || | ||
| bytes.readUInt16LE(2) !== 8 | ||
| ) | ||
| return err({ | ||
| kind: "unsupported", | ||
| message: "The AndroidManifest.xml member is not a binary XML document.", | ||
| }); | ||
| const reader = new BinaryReader(bytes, true); | ||
| const declared = reader.u32(4); | ||
| if (declared < 8 || declared > bytes.length) | ||
| return err({ | ||
| kind: "malformed", | ||
| message: "The binary XML document declares a size outside the member.", | ||
| }); | ||
| const chunks: { | ||
| type: number; | ||
| start: number; | ||
| headerSize: number; | ||
| chunkSize: number; | ||
| }[] = []; | ||
| let start = 8; | ||
| while (start + 8 <= declared) { | ||
| const chunk = { | ||
| type: reader.u16(start), | ||
| headerSize: reader.u16(start + 2), | ||
| start, | ||
| chunkSize: reader.u32(start + 4), | ||
| }; | ||
| if ( | ||
| chunk.chunkSize < 8 || | ||
| chunk.headerSize < 8 || | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For a malformed APK whose AGENTS.md reference: AGENTS.md:L21-L21 Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in |
||
| start + chunk.chunkSize > declared | ||
| ) | ||
| return err({ | ||
| kind: "malformed", | ||
| message: "A binary XML chunk declares a size outside the document.", | ||
| }); | ||
| chunks.push(chunk); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
On an admitted 4 MiB manifest containing a valid pool/root plus many minimum-size chunks, this loop can retain about 524k JavaScript chunk objects before any manifest lookup; the later filter/map passes add more arrays, so the nominal byte cap expands into tens of megabytes and avoidable CPU even though the result only needs the root. Process chunks incrementally or impose a representation/count limit. AGENTS.md reference: AGENTS.md:L57-L58 Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in |
||
| start += chunk.chunkSize; | ||
| } | ||
| const poolChunk = chunks.find( | ||
| (chunk) => chunk.type === RES_STRING_POOL_TYPE, | ||
| ); | ||
| if (poolChunk === undefined) | ||
| return err({ | ||
| kind: "malformed", | ||
| message: "The binary manifest has no string pool.", | ||
| }); | ||
| const pool = readStringPool( | ||
| reader, | ||
| poolChunk.start, | ||
| poolChunk.headerSize, | ||
| poolChunk.chunkSize, | ||
| ); | ||
| if (!pool.ok) return pool; | ||
| const strings = pool.value; | ||
| const resourceIds: number[] = []; | ||
| for (const chunk of chunks) { | ||
| if (chunk.type !== RES_XML_RESOURCE_MAP_TYPE) continue; | ||
| for ( | ||
| let offset = chunk.start + chunk.headerSize; | ||
| offset + 4 <= chunk.start + chunk.chunkSize; | ||
| offset += 4 | ||
| ) | ||
| resourceIds.push(reader.u32(offset)); | ||
| break; | ||
| } | ||
| const manifest = chunks | ||
| .filter((chunk) => chunk.type === RES_XML_START_ELEMENT_TYPE) | ||
| .map((chunk) => { | ||
| const el = readStartElement( | ||
| reader, | ||
| strings, | ||
| resourceIds, | ||
| chunk.start, | ||
| chunk.headerSize, | ||
| chunk.chunkSize, | ||
| ); | ||
| return el; | ||
| }) | ||
| .find((element) => element?.name === "manifest"); | ||
| if (manifest === undefined || manifest === null) | ||
| return err({ | ||
| kind: "malformed", | ||
| message: "The binary manifest has no <manifest> start element.", | ||
| }); | ||
| const android = manifest.attributes.filter( | ||
| (attribute) => attribute.namespace === ANDROID_NS, | ||
| ); | ||
| const toValue = (attribute: (typeof android)[number]): string | null => | ||
| attribute.dataType === TYPE_STRING | ||
| ? (strings[attribute.data] ?? null) | ||
| : attribute.raw; | ||
| const find = (resourceId: number, name: string) => | ||
| android.find( | ||
| (attribute) => | ||
| attribute.resourceId === resourceId || attribute.name === name, | ||
| ); | ||
| const requiredSplitTypes = find( | ||
| REQUIRED_SPLIT_TYPES_ID, | ||
| "requiredSplitTypes", | ||
| ); | ||
| const splitName = find(SPLIT_NAME_ID, "splitName"); | ||
| const requiredFlag = find(IS_SPLIT_REQUIRED_ID, "isSplitRequired"); | ||
| const isSplitRequired = | ||
| requiredFlag === undefined | ||
| ? null | ||
| : requiredFlag.dataType === TYPE_INT_BOOLEAN | ||
| ? requiredFlag.data !== 0 | ||
| : requiredFlag.dataType === TYPE_STRING | ||
| ? (strings[requiredFlag.data] ?? null) === "true" | ||
| ? true | ||
| : (strings[requiredFlag.data] ?? null) === "false" | ||
| ? false | ||
| : null | ||
| : null; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a valid manifest encodes AGENTS.md reference: AGENTS.md:L21-L21 Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in |
||
| const requiredValue = | ||
| requiredSplitTypes === undefined ? null : toValue(requiredSplitTypes); | ||
| return ok({ | ||
| required_split_types: | ||
| requiredValue === null ? null : splitTypes(requiredValue), | ||
| split_name: splitName === undefined ? null : toValue(splitName), | ||
| is_split_required: isSplitRequired, | ||
| }); | ||
| } catch (cause: unknown) { | ||
| if (cause instanceof BinaryFormatFailure) | ||
| return err({ | ||
| kind: cause.kind, | ||
| message: cause.message, | ||
| }); | ||
| throw cause; | ||
| } | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a string-pool offset or encoding is malformed, or the cumulative string budget is exhausted,
fail()insertsnulland the parser can still returnok; if that entry is the Android namespace, split attribute name, or split value, later code maps it to an absent declaration and the caller emits no inventory unknown. Preserve a decode/capacity limitation and fail or qualify the result when it affects the split facts.AGENTS.md reference: AGENTS.md:L37-L37
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed on the failure mode: a string-pool entry lost to a malformed offset or to the cumulative allowance can leave a split attribute's name, namespace or value null, and the caller then reports an absent declaration. Not fixed in this push — the qualification needs a distinct marker from readStringPool through to the attribute resolution, and I did not want to land that half-verified. I will follow up on this PR with the capacity/malformed distinction and a boundary test that exhausts the string allowance on a retained split value.