Skip to content

CODEC-315: Fix possible IndexOutOfBoundException thrown by PhoneticEngine.encode method#223

Merged
garydgregory merged 1 commit into
apache:masterfrom
arthurscchan:CODEC-315-IndexOutOfBound
Nov 25, 2023
Merged

CODEC-315: Fix possible IndexOutOfBoundException thrown by PhoneticEngine.encode method#223
garydgregory merged 1 commit into
apache:masterfrom
arthurscchan:CODEC-315-IndexOutOfBound

Conversation

@arthurscchan

@arthurscchan arthurscchan commented Nov 23, 2023

Copy link
Copy Markdown
Contributor

This fixes possible StringIndexOutOfBoundsException and ArrayIndexOutOfBoundsException in src/main/java/org/apache/commons/codec/language/bm/PhoneticEngine.java thrown by PhoneticEngine.encode() when the provided string only contains one of the name prefix of the chosen NameType or only contain single quotation character.

This PR fixes the parameter for the split method and adds a conditional check to ensure only strings and arrays are not empty before processing.

We found this bug using fuzzing by way of OSS-Fuzz. It is reported at https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=64376 and https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=64395.

@garydgregory garydgregory left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@arthurscchan
Thank you for your PR.
Please see my comments.

Comment thread src/test/java/org/apache/commons/codec/language/bm/PhoneticEngineTest.java Outdated
Comment thread src/main/java/org/apache/commons/codec/language/bm/PhoneticEngine.java Outdated
@garydgregory

Copy link
Copy Markdown
Member

@arthurscchan
Please use a better description in PRs and JIRA: Specify the class and method where the exception occurs.

@arthurscchan arthurscchan force-pushed the CODEC-315-IndexOutOfBound branch from 7a6ce1d to f6ab92f Compare November 24, 2023 19:51
Signed-off-by: Arthur Chan <arthur.chan@adalogics.com>
@arthurscchan arthurscchan force-pushed the CODEC-315-IndexOutOfBound branch from f6ab92f to ee02460 Compare November 24, 2023 20:40
@arthurscchan arthurscchan changed the title CODEC-315: Fix possible IndexOutOfBoundException CODEC-315: Fix possible IndexOutOfBoundException thrown by PhoneticEngine.encode method Nov 24, 2023
@codecov-commenter

codecov-commenter commented Nov 25, 2023

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 92.27%. Comparing base (44e4c4d) to head (ee02460).
⚠️ Report is 656 commits behind head on master.

Files with missing lines Patch % Lines
...ache/commons/codec/language/bm/PhoneticEngine.java 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             master     #223   +/-   ##
=========================================
  Coverage     92.27%   92.27%           
- Complexity     1742     1743    +1     
=========================================
  Files            67       67           
  Lines          4584     4585    +1     
  Branches        709      710    +1     
=========================================
+ Hits           4230     4231    +1     
  Misses          242      242           
  Partials        112      112           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@garydgregory garydgregory merged commit 1f908b2 into apache:master Nov 25, 2023
asfgit pushed a commit that referenced this pull request Nov 25, 2023
omosteven pushed a commit to omosteven/commons-codec-lab-work that referenced this pull request Jan 8, 2025
Signed-off-by: Arthur Chan <arthur.chan@adalogics.com>
omosteven pushed a commit to omosteven/commons-codec-lab-work that referenced this pull request Jan 8, 2025
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.

3 participants