Fix MD5 checksum encoding dropping leading zeros in HashUtils - #1400
Open
UditDewan wants to merge 1 commit into
Open
Fix MD5 checksum encoding dropping leading zeros in HashUtils#1400UditDewan wants to merge 1 commit into
UditDewan wants to merge 1 commit into
Conversation
HashUtils encoded MD5 digests with BigInteger(1, digest).toString(16),
which interprets the digest as a number and drops leading zeros. Any
digest beginning with one or more zero nibbles produced a hex string
shorter than the expected 32 characters, e.g. MD5("a") rendered as
"cc175b9c0f1b6a831c399e269772661" (31 chars) and MD5("jk8ssl") as
"18e6137ac2caab16074784a6" (24 chars). Roughly one file in sixteen has
a digest starting with a zero nibble, so those checksums silently did
not match the canonical MD5 value.
Use AppEventUtility.bytesToHex (same package), which formats each byte
as two hex digits and therefore preserves leading zeros, for both the
file digest and the PackageManager checksum paths. Add a regression test
covering single- and multi-zero-nibble digests.
Co-authored-by: Baradhan-Madhu <26barum@gmail.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
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.
Summary
HashUtilshex-encodes MD5 digests with:BigInteger.toString(16)treats the digest as a number, so it drops leading zeros. Any digest that begins with one or more zero nibbles produces a hex string shorter than the expected 32 characters:BigIntegerproduced"a"0cc175b9c0f1b6a831c399e269772661cc175b9c0f1b6a831c399e269772661(31 chars)"jk8ssl"0000000018e6137ac2caab16074784a618e6137ac2caab16074784a6(24 chars)Roughly one digest in sixteen starts with a zero nibble, so a meaningful fraction of checksums silently did not equal the canonical MD5 value. The same flaw was present in both
computeFileMd5and thePackageManager.requestChecksumspath.Change
Use
AppEventUtility.bytesToHex— already defined in the same package — which formats each byte as exactly two hex digits (%02x) and therefore preserves leading zeros. This also removes the now-unusedBigIntegerimport.Tests
Added a regression test in
HashUtilsTestcovering a single leading-zero nibble ("a") and many leading-zero nibbles ("jk8ssl"), both of which failed before this change. Existing non-zero-prefixed cases are unchanged.