Skip to content

Conversation

@bdrodes
Copy link
Contributor

@bdrodes bdrodes commented Oct 24, 2025

Weak symmetric cipher looks to see if the algorithm is not AES but is missing a type check to limit to just symmetric ciphers.

…er if !=AES but the algorithm must still be a SymmetriCipher algorithm.
@bdrodes bdrodes requested a review from a team as a code owner October 24, 2025 12:19
Copilot AI review requested due to automatic review settings October 24, 2025 12:19
Copy link
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

This PR fixes a bug in the weak symmetric cipher detection query where the logic was incorrectly flagging non-symmetric cipher algorithms. The fix adds a type check to ensure only symmetric ciphers are evaluated.

  • Added type constraint to limit detection to symmetric ciphers only
  • Added test case with PBKDF2WithHmacSHA256 (a key derivation function, not a symmetric cipher) to verify the fix
  • Updated test expectations with line number adjustments due to the new test case

Reviewed Changes

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

File Description
WeakSymmetricCipher.qll Added instanceof TSymmetricCipher constraint to ensure only symmetric cipher algorithms are checked
Test.java Added test case using SecretKeyFactory with PBKDF2WithHmacSHA256 to verify non-symmetric algorithms are not flagged
WeakSymmetricCipher.expected Updated expected test results with adjusted line numbers and added numbering prefix

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@github-actions github-actions bot added the Java label Oct 24, 2025
@nicolaswill nicolaswill merged commit d478752 into github:main Oct 24, 2025
14 checks passed
@nicolaswill nicolaswill deleted the weak_symmetric_cipher_bug branch October 24, 2025 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants