Skip to content

Add EC to secure algorithm whitelist for Java CWE-327 query#21594

Merged
owen-mc merged 4 commits intogithub:mainfrom
MarkLee131:fix/add-ec-to-secure-algorithm-whitelist
Mar 28, 2026
Merged

Add EC to secure algorithm whitelist for Java CWE-327 query#21594
owen-mc merged 4 commits intogithub:mainfrom
MarkLee131:fix/add-ec-to-secure-algorithm-whitelist

Conversation

@MarkLee131
Copy link
Copy Markdown
Contributor

fix #21593

@MarkLee131 MarkLee131 requested a review from a team as a code owner March 27, 2026 11:13
Copilot AI review requested due to automatic review settings March 27, 2026 11:13
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the Java CWE-327 “potentially weak cryptographic algorithm” query support code to treat common Elliptic Curve (EC) algorithm names as secure, addressing reported false positives.

Changes:

  • Extend the secure-algorithm whitelist in Encryption.qll to include EC/ECDH/ECDSA and related modern curve families (EdDSA/Ed25519/Ed448, XDH/X25519/X448).
  • Add query-test coverage examples intended to ensure these algorithms are no longer flagged.
  • Add a change note documenting the reduction in false positives.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
java/ql/lib/semmle/code/java/security/Encryption.qll Expands the secure algorithm name whitelist to include EC-family algorithm identifiers.
java/ql/test/query-tests/security/CWE-327/semmle/tests/Test.java Adds “GOOD” examples for EC-family algorithm strings to prevent regressions in CWE-327 behavior.
java/ql/lib/change-notes/2026-03-27-add-ec-to-secure-algorithms.md Documents the analysis-result change (fewer false positives).

Copy link
Copy Markdown
Contributor

@owen-mc owen-mc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this contribution. It looks good, and should be mergeable with a few small changes. Please add signature.getInstance as a sink, as I explained in another comment.

In another forum, a user complained about FPs with this query, and gave the below examples. Is it possible you could add them at the same time?

import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import javax.crypto.spec.SecretKeySpec;
import javax.crypto.SecretKeyFactory;
import java.security.KeyPairGenerator;

public class C {


    public static void main(String[] args) {

        try {
            MessageDigest msgDigester1 = MessageDigest.getInstance("MD5");     //OK
            MessageDigest msgDigester2 = MessageDigest.getInstance("SHA-1");   //OK
            MessageDigest msgDigester3 = MessageDigest.getInstance("SHA-256"); //OK

            new SecretKeySpec(null, "HMACSHA1"); // FP
            new SecretKeySpec(null, "HMACSHA256"); // FP
            new SecretKeySpec(null, "HMACSHA384"); // FP
            new SecretKeySpec(null, "SHA384withECDSA"); // FP

            SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1");  // FP

            KeyPairGenerator.getInstance("EC");  // FP


        } catch (NoSuchAlgorithmException nsae) {
            System.out.println("NoSuchAlgorithmException caught when digest token");
        }

    }
}

@MarkLee131
Copy link
Copy Markdown
Contributor Author

no problem. I will handle all these issues you mentioned ASAP. Will let u know when ready. :)

…ist, fix test APIs

- Model Signature.getInstance() as CryptoAlgoSpec sink (previously only
  Signature constructor was modeled)
- Add HMAC-based algorithms (HMACSHA1/256/384/512, HmacSHA1/256/384/512)
  and PBKDF2 to the secure algorithm whitelist
- Fix XDH/X25519/X448 tests to use KeyAgreement.getInstance() instead of
  KeyPairGenerator.getInstance() to match their key agreement semantics
- Add test cases for SHA384withECDSA, HMACSHA*, and PBKDF2WithHmacSHA1
  from user-reported false positives
- Update change note to document all additions
@MarkLee131 MarkLee131 force-pushed the fix/add-ec-to-secure-algorithm-whitelist branch from 80880b4 to da4a223 Compare March 28, 2026 08:54
@MarkLee131
Copy link
Copy Markdown
Contributor Author

Added all examples. Also added HMAC/PBKDF2 to whitelist and updated change note.

@MarkLee131 MarkLee131 requested a review from owen-mc March 28, 2026 09:02
@owen-mc owen-mc merged commit 58218ee into github:main Mar 28, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

False positive: java/potentially-weak-cryptographic-algorithm flags secure Elliptic Curve algorithms (EC, ECDSA, ECDH, EdDSA, etc.)

3 participants