Skip to content

Ed25519 and Ed448 generate() rules were matching too broadly#429

Open
Arijit429 wants to merge 1 commit into
cbomkit:mainfrom
Arijit429:fix/python-false-positive-ed25519-ed448
Open

Ed25519 and Ed448 generate() rules were matching too broadly#429
Arijit429 wants to merge 1 commit into
cbomkit:mainfrom
Arijit429:fix/python-false-positive-ed25519-ed448

Conversation

@Arijit429
Copy link
Copy Markdown

Fixes #300

The generate() rules for Ed25519 and Ed448 were using withAnyParameters(),
which caused the scanner to match any .generate() call regardless of parameters.

The actual Ed25519PrivateKey.generate() and Ed448PrivateKey.generate() from
pyca/cryptography take no parameters. So withoutParameters() is the correct matcher.

Reproducer from the issue — this was getting flagged as Edwards448 Key Pair Generation:

generated_ids = self.vlm_model.generate(
    **inputs,
    max_new_tokens=self.max_new_tokens,
)

Changed withAnyParameters() to withoutParameters() in both SIGN_ED25519
and SIGN_ED448 rules in PycaSign.java.

@Arijit429 Arijit429 requested a review from a team as a code owner May 23, 2026 04:08
…prevent false positives

Signed-off-by: Arijit429 <arijitdeb1203@gmail.com>
@Arijit429 Arijit429 force-pushed the fix/python-false-positive-ed25519-ed448 branch from eeff0b7 to d27f2f5 Compare May 23, 2026 04:11
@san-zrl san-zrl self-assigned this May 29, 2026
Copy link
Copy Markdown
Contributor

@san-zrl san-zrl left a comment

Choose a reason for hiding this comment

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

You claim to fix #300. This is not quite correct.

When scanning https://github.com/docling-project/docling the original version detects 48 assets all of which seem to be false positives (see dockling-cbom-original.json). All detection relates to a "generate" context.

After applying your PR the scan detects 24 assets (see dockling-cbom-with-pr.json). These findings also relate entirely to a "generate" call on some object.

The reduced the number of FPs tells us

  • The fix is correct as it solves some of the observed problems.
  • Your PR only partially addresses #300. You must have missed some other detection rules with matching conditions which are too general

If you'd like to continue working on this PR please folllow the advice below. I'm happy to review the next version.

  1. Follow the description in the issue on how to reproduce the problem in the issue. Good issues should contain such a description. Apply your PR and verify if your claim to fix the targeted issue is correct.
  2. Your PR does not contain any regression tests. In this example it should have test cases for the positive case (finding correctly detected) and the negative case (no false positive).

@Arijit429
Copy link
Copy Markdown
Author

You claim to fix #300. This is not quite correct.

When scanning https://github.com/docling-project/docling the original version detects 48 assets all of which seem to be false positives (see dockling-cbom-original.json). All detection relates to a "generate" context.

After applying your PR the scan detects 24 assets (see dockling-cbom-with-pr.json). These findings also relate entirely to a "generate" call on some object.

The reduced the number of FPs tells us

  • The fix is correct as it solves some of the observed problems.
  • Your PR only partially addresses Python Scanner generates False Positives #300. You must have missed some other detection rules with matching conditions which are too general

If you'd like to continue working on this PR please folllow the advice below. I'm happy to review the next version.

  1. Follow the description in the issue on how to reproduce the problem in the issue. Good issues should contain such a description. Apply your PR and verify if your claim to fix the targeted issue is correct.
  2. Your PR does not contain any regression tests. In this example it should have test cases for the positive case (finding correctly detected) and the negative case (no false positive).

Thank you for the detailed review and for running the scan yourself.

You're right — I only looked at Ed25519 and Ed448 but didn't check the
other rules using withAnyParameters(). I'll go through all the Python
detection rules, find the remaining ones with overly broad matching,
fix them, and add the regression tests you mentioned.

Will push an update shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python Scanner generates False Positives

2 participants