[jankesec]

Closes #3933

Description

Adds an Android counterpart to MASTG-TEST-0211: Broken Hashing Algorithms.

MASTG-TEST-0014 (v1) is currently marked as covered only by MASTG-TEST-0312, which checks explicit JCA providers and does not evaluate the selected hashing algorithm. A call such as MessageDigest.getInstance("MD5") is therefore out of scope for the existing v2 tests.

This PR adds:

• MASTG-TEST-0x01: static/code/manual test mapped to MASWE-0008, with the required prerequisites, knowledge, and best-practice references
• Semgrep rule mastg-android-broken-hashing-algorithms for reversed Java: case-insensitive MessageDigest.getInstance matching, including provider overloads and common aliases, plus Guava Hashing and Apache Commons DigestUtils wrappers
• MASTG-DEMO-0x01: MASTestApp sample, jadx-reversed Java, Semgrep output, and the exact validation environment
• Android knowledge and best-practice pages covering JCA message digests and appropriate hash selection
• covered_by update on MASTG-TEST-0014

Fake IDs (MASTG-TEST-0x01, MASTG-DEMO-0x01, MASTG-KNOW-0x01, and MASTG-BEST-0x01) follow the project instructions until real IDs are assigned before merge.

The demo APK was built with MASTestApp, installed on a Pixel 7 Android Virtual Device running Android 13, and exercised via the Start button. The rule was also checked with lowercase and mixed-case algorithm names, provider overloads, fully qualified receivers, Guava and Apache Commons wrappers, and a SHA-256 negative control. It reports the five broken-hash cases and does not flag SHA-256. Running it on the stored reversed Java reports the expected four findings with no errors.

AI Tool Disclosure

Check exactly one option.

• This contribution does not include AI-generated content.

• This contribution includes AI-generated content.

• AI tools used: xAI Grok and OpenAI Codex

• Models and versions: Grok 4.6 and GPT-6

• Prompt summary: Draft an Android counterpart to MASTG-TEST-0211 using the MASTG test, demo, and rule writing instructions; validate the sample on MASTestApp reversed Java. Review PR Add Android counterpart to MASTG-TEST-0211 for broken hashing algorithms #3952, implement the remaining review feedback, verify the Semgrep rule with positive and negative controls, run the project checks, and update the PR.

• Your mobile security expertise level: high

Author of this PR and all commits: Sevban Dönmez (@jankesec). I reviewed, edited, built, and ran the demo. Commit history has an initial content commit followed by review commits.

Contributor Checklist

• I have read and understood the contributing guidelines.
• I followed the project style guide.
• I validated the technical correctness of my changes and understand the topic.
• This PR adds clear value and is not spam or low-effort content.

[github-actions]

Thank you for your contribution! However, this PR does not reference any issues.

To contribute to this project, please:

1. Find an existing issue you'd like to work on, or create a new one
2. Get assigned to the issue
3. Reference the issue in your PR using keywords like "Fixes 0x4 - About Burp Suite #123" or "Closes 0x4 - About Burp Suite #123"

Please reopen this PR once you have linked it to an issue you are assigned to.

[phplego]

Thanks for addressing #3933! A few points before review:

• PR closure: the bot reported no linked issue, although the description currently includes Closes #3933. Could a maintainer check the linkage and assign @jankesec so the PR can be reopened?
• MD5: please remove “preimage attacks are practical.” RFC 6151 §2.2 describes a preimage attack costing 2^123.4 operations. Practical collisions and password guessing should be distinguished from this.
• SHA-1: for practical chosen-prefix collisions, please cite SHA-1 is a Shambles. SHAttered demonstrated an identical-prefix collision.
• Demo: consider hashing ordinary data in the SHA-256 PASS example. It passes this algorithm check, but raw SHA-256 is unsuitable for password storage. Likewise, label the non-security MD5 checksum as requiring contextual validation rather than an unconditional FAIL.

Thanks again for filling this Android coverage gap.

[jankesec]

Thanks @cpholguera for reopening the PR!

And thank you @phplego for the thorough, high-quality cryptographic review:

1. MD5: Updated MASTG-TEST-0x01.md to distinguish practical collision attacks from theoretical preimage attacks, explicitly citing RFC 6151 §2.2 regarding preimage complexity (~2^123.4 operations) and highlighting practical dictionary / rainbow-table attacks for credential scenarios.
2. SHA-1: Updated the chosen-prefix collision citation to reference SHA-1 is a Shambles while clarifying SHAttered for identical-prefix collisions.
3. Demo & PASS Example:
   • Updated the SHA-256 PASS case in MastgTest.kt and MastgTest_reversed.java to hash ordinary document data (readme.txt) instead of a password, aligning with the OWASP Password Storage Cheat Sheet.
   • Clarified in both the code comments and MASTG-DEMO-0x01.md evaluation that non-security MD5 checksums require contextual validation rather than being treated as an unconditional failure.

Pushed these changes in commit 21df7dc.

[Copilot]

🟡 Changes recommended

Three moderate Semgrep-rule findings remain, along with documentation and metadata nits.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds an Android counterpart for broken hashing algorithm detection, including a Semgrep rule, test documentation, demo artifacts, and updated coverage metadata.

Changes:

• Adds MASTG-TEST-0x01 mapped to MASWE-0008.
• Adds detection for broken hashing APIs and a MASTestApp demo.
• Updates MASTG-TEST-0014 coverage references.

File summaries

File	Changes and final review findings
tests/android/MASVS-CRYPTO/MASTG-TEST-0014.md	Updates coverage references. No final comments.
tests-beta/android/MASVS-CRYPTO/MASTG-TEST-0x01.md	Adds the Android hashing test. Nit (3 votes): add required metadata and prerequisites. Nit (1 vote): clarify the security-context condition in the failure sentence. Nit (1 vote): use Hashing.md5 and Hashing.sha1 without parentheses.
rules/mastg-android-broken-hashing-algorithms.yaml	Adds Semgrep detections. Moderate (2 votes): support case-insensitive algorithm names and provider overloads. Moderate (1 vote): use the correct Apache Commons getMd5Digest() and getSha1Digest() methods. Moderate (1 vote): support qualified API receivers.
demos/android/MASVS-CRYPTO/MASTG-DEMO-0x01/run.sh	Runs Semgrep. No final comments.
demos/android/MASVS-CRYPTO/MASTG-DEMO-0x01/output.txt	Stores sample findings. No final comments.
demos/android/MASVS-CRYPTO/MASTG-DEMO-0x01/MastgTest.kt	Provides vulnerable and safe examples. No final comments.
demos/android/MASVS-CRYPTO/MASTG-DEMO-0x01/MastgTest_reversed.java	Provides reversed Java sample code. No final comments.
demos/android/MASVS-CRYPTO/MASTG-DEMO-0x01/MASTG-DEMO-0x01.md	Documents the demo. Nit (2 votes): add Android/platform, IDE, and device environment details.

Review details

Suppressed comments (4)

rules/mastg-android-broken-hashing-algorithms.yaml:31

• These are not the Apache Commons Codec factory method names: DigestUtils exposes getMd5Digest() and getSha1Digest(). As written, the rule misses code that obtains a broken digest through those common wrappers and includes patterns that cannot match the actual API; replace these patterns with the get...Digest() calls.

      - pattern: DigestUtils.md5Digest(...)
      - pattern: DigestUtils.sha1Digest(...)

rules/mastg-android-broken-hashing-algorithms.yaml:14

• These patterns only match a bare MessageDigest receiver, so a valid fully qualified call such as java.security.MessageDigest.getInstance("MD5") is missed by the rule; the same limitation applies to the Hashing and DigestUtils alternatives below. That creates a false negative for an API this test claims to cover, so add qualified-name or type-aware patterns and cover the form in the demo or document it as a limitation.

      - pattern: MessageDigest.getInstance("MD2")
      - pattern: MessageDigest.getInstance("MD2", ...)
      - pattern: MessageDigest.getInstance("MD4")
      - pattern: MessageDigest.getInstance("MD4", ...)
      - pattern: MessageDigest.getInstance("MD5")

tests-beta/android/MASVS-CRYPTO/MASTG-TEST-0x01.md:45

• The failure sentence treats every occurrence as a failure, but the following block explicitly permits non-security checksums and requires contextual validation. State the security-relevant-context condition here so the test's evaluation does not contradict its own checksum exception.

The test case fails if you can find the use of broken hashing algorithms. For example:

tests-beta/android/MASVS-CRYPTO/MASTG-TEST-0x01.md:30

• These method names are inline identifiers, so the backticks should omit the call parentheses (Hashing.md5 and Hashing.sha1), per the project's Markdown identifier rules. Keeping them as Hashing.md5() and Hashing.sha1() violates the enforced style.

Third-party wrappers such as Guava `Hashing.md5()` / `Hashing.sha1()` and Apache Commons Codec `DigestUtils` may call the same algorithms. Native libraries (for example OpenSSL `MD5_*` / `SHA1_*`) can implement hashing without going through `MessageDigest`.

• Files reviewed: 8/8 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[TheDauntless]

Thanks for contributing! I've added some comments, can you take a look?

[TheDauntless]

This doesn't appear to be true: getAlgorithm returns whatever you specified in getInstance; not the canonical name.

[TheDauntless]

SHA1 has been disallowed for cryptographic signatures (so not 'all cryptographic applications') with a total ban end of 2030:

"NIST has announced the retirement of SHA-1, with a phase-out deadline of 31 December 2030."

[TheDauntless]

Does it make sense to have MD2 and MD4, which aren't supported on Android? (MD4 is included in the regexes)

[TheDauntless]

Also only available on specific versions of Android

[TheDauntless]

The code hashes the string readme.txt, not the content of the file. This is not in line with what the documentation / test evaluation says.