Skip to content

APPSEC-2489 SONARJAVA-6939 Add CryptographicKeySizeConfiguration to appsec package - #528

Merged
asya-vorobeva merged 2 commits into
masterfrom
asya/add-cryptographic-keys-configurator
Sep 15, 2026
Merged

asya-vorobeva merged 2 commits into
masterfrom
asya/add-cryptographic-keys-configurator

Conversation

@asya-vorobeva

Copy link
Copy Markdown
Contributor

Extracts language-agnostic crypto key-size configuration logic into sonar-analyzer-commons so it can be reused by other language analyzers. Provides default minimum key sizes, a configurable parse/merge API, and EC curve name key-size extraction.

…ppsec package

Extracts language-agnostic crypto key-size configuration logic into sonar-analyzer-commons so it can be reused by other
language analyzers. Provides default minimum key sizes, a configurable
parse/merge API, and EC curve name key-size extraction.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

APPSEC-2489
SONARJAVA-6939

@gitar-bot

gitar-bot Bot commented Sep 15, 2026 •

Copy link
Copy Markdown
Code Review ✅ Approved 3 resolved / 3 findings

Extracts language-agnostic crypto key-size configuration logic into sonar-analyzer-commons for reuse across language analyzers. Resolved NumberFormatException on long digit runs in extractEcKeySize and fixed EC curve pattern case-sensitivity to align with the rest of the API.

✅ 3 resolved
✅ Edge Case: extractEcKeySize throws NumberFormatException on long digit runs

📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/CryptographicKeySizeConfiguration.java:102-107
EC_CURVE_KEY_PATTERN captures (\d+) with no length bound, so any input whose digit run exceeds Integer.MAX_VALUE — e.g. a string literal "secp99999999999" taken from analyzed source code — makes Integer.parseInt(matcher.group(2)) throw an uncaught NumberFormatException instead of returning OptionalInt.empty() as the javadoc promises; in a rule that feeds source-code strings into this helper, that aborts analysis of the file. Note that parseKeySizes already guards its parseInt with a try/catch, so the two entry points are inconsistent. Bound the digit group or catch the exception.

✅ Bug: EC curve pattern is case-sensitive, unlike rest of the API

📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/CryptographicKeySizeConfiguration.java:31 📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/CryptographicKeySizeConfiguration.java:49 📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/CryptographicKeySizeConfiguration.java:102-107
EC_CURVE_KEY_PATTERN has no Pattern.CASE_INSENSITIVE flag, so extractEcKeySize("SECP256R1") or extractEcKeySize("Prime256v1") returns empty even though those spellings are accepted by JCA providers (ECGenParameterSpec name lookup is case-insensitive) and will appear in analyzed code — a silent false negative for a key-size rule. This also contradicts the class javadoc's case-insensitivity promise and the convention in the sibling appsec classes, which compile every source-derived pattern with CASE_INSENSITIVE.

✅ Edge Case: EC prefix list omits c2pnb/c2onb X9.62 binary curves

📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/CryptographicKeySizeConfiguration.java:49 📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/CryptographicKeySizeConfiguration.java:96-97
The prefix alternation covers c2tnb but not the sibling X9.62 binary-field families c2pnb and c2onb (e.g. c2pnb163v1, c2pnb176w1, c2onb191v4), which are valid curve names in common providers such as Bouncy Castle. extractEcKeySize("c2pnb163v1") therefore returns empty and a consuming rule cannot flag those weak curves. Add the missing prefixes (or document the omission explicitly if it is intentional).

Review coverage

Functional validation No results

Rules No rules evaluated

Auto-approval Not enabled · Set up

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqube-next

Copy link
Copy Markdown

@aurelien-coet-sonarsource aurelien-coet-sonarsource left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@asya-vorobeva
asya-vorobeva merged commit 3bbfd77 into master Sep 15, 2026
13 of 14 checks passed
@asya-vorobeva
asya-vorobeva deleted the asya/add-cryptographic-keys-configurator branch September 15, 2026 12:56
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.

2 participants