Add ASCII-only character validation for enhanced security#167
Merged
hajk1 merged 1 commit intoarturmkrtchyan:masterfrom Sep 28, 2025
Merged
Conversation
Contributor
Author
|
Hi @hajk1 — sorry to ping you out of the blue. You appear to be among the most recent active contributors (based on PRs). Could you advise on whether the PR aligns with the project’s direction and if there’s a path to merge or close it? Happy to adjust per your guidance. Thanks! |
- Created CharacterUtil with strict ASCII validation methods - Replaced Character.isDigit() with IbanCharacterUtil.isAsciiDigit() - Replaced Character.isUpperCase() with IbanCharacterUtil.isAsciiUppercaseLetter() - Replaced Character.isLetterOrDigit() with IbanCharacterUtil.isValidIbanAlphanuc) - Improves performance with direct range checks vs Unicode lookups - Added comprehensive tests for new validation methods - All existing tests continue to pass
RedSlowpoke
force-pushed
the
ibans-only-latin-and-common-digits
branch
from
92fe553 to
5b14c70
Compare
RedSlowpoke
commented
Sep 14, 2025
Comment on lines
-110
to
+116
| if (countryCode.trim().length() < COUNTRY_CODE_LENGTH | ||
| || !countryCode.equals(countryCode.toUpperCase()) | ||
| || !Character.isLetter(countryCode.charAt(0)) | ||
| || !Character.isLetter(countryCode.charAt(1))) { | ||
| throw new BicFormatException( | ||
| COUNTRY_CODE_ONLY_UPPER_CASE_LETTERS, | ||
| countryCode, | ||
| "Bic country code must contain upper case letters"); | ||
| for (int i = 0; i < COUNTRY_CODE_LENGTH; i++) { | ||
| if (!CharacterUtil.isAsciiUppercaseLetter(countryCode.charAt(i))) { | ||
| throw new BicFormatException( | ||
| COUNTRY_CODE_ONLY_UPPER_CASE_LETTERS, | ||
| countryCode, | ||
| "Bic country code must contain upper case letters"); | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Use of magic indexes removed — validation now uses a loop for the expected lenght.
The length check removed, since it doesn't correspond with the exception in this check.
Country code lenght/validity effectively is ensured by the check against known country list check below.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale
IBANs and BICs should only contain ASCII digits (0-9) and Latin uppercase letters (A-Z). The current implementation uses Java's
Character.isDigit()andCharacter.isUpperCase()methods, which accept a much wider range of Unicode characters than necessary.This creates potential security vulnerabilities in real-world scenarios. For example:
Solution
Created a new
CharacterUtilclass with strict ASCII-only validation methods:isAsciiDigit(char)- validates only ASCII digits 0-9isAsciiUppercaseLetter(char)- validates only ASCII uppercase letters A-ZisValidAlphanumeric(char)- validates IBAN-compliant alphanumeric charactersReplaced existing
Character.*method calls in:BbanStructure.java- digit and letter validationIbanUtil.java- check digit validationBicUtil.java- check digit and letter validation. Additionnaly change invalidateCountryCodelogic to not use magic values and make the check correspond with the thrown exception.Benefits
The solution is simple, transparent, and maintains performance