kernel: check kernel_read() results when verifying the APK signing block#3594
Open
palazik wants to merge 1 commit into
Open
kernel: check kernel_read() results when verifying the APK signing block#3594palazik wants to merge 1 commit into
palazik wants to merge 1 commit into
Conversation
check_block() and check_v2_signature() walk the APK signing block by issuing a series of kernel_read() calls and never look at whether they actually read what they asked for. On a truncated or malformed APK the subsequent offset math runs on stale/garbage stack values. A valid manager APK reads back every field in full, so behavior there is unchanged; a short read now just bails out and treats the APK as not matching. The EOCD scan loop is left as-is on purpose.
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
1 participant
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.
check_block()andcheck_v2_signature()inapk_sign.cread the APK v2 signing block field by field withkernel_read(), but none of those calls check how many bytes actually came back. On a truncated or otherwise malformed APK the code keeps doing offset arithmetic on whatever happened to be on the stack.It's bounded by the
CERT_MAX_LENGTHguard so this isn't a memory-safety hole, but relying on uninitialized/stale values on the manager-verification path isn't great.This adds the length checks:
check_block, every field read plus the certificate read now bail out (return false) on a short read;check_v2_signature, the reads after the EOCD scangoto clean, and the two reads inside the id-block loop invalidate the result first.A properly signed manager APK reads every field back in full, so nothing changes for the normal case — only truncated/garbage input is now rejected explicitly instead of processed. I deliberately left the EOCD-search loop untouched since it already tolerates short reads via its
n == icheck.