Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The constructor of com.bazaarvoice.jolt.common.pathelement.StarDoublePathElement class could throw an unexpected ArrayIndexOutOfBoundsException #1268

Open
arthurscchan opened this issue Aug 1, 2024 · 0 comments

Comments

@arthurscchan
Copy link

Bug description

    public StarDoublePathElement(String key) {
        super(key);

        if ( StringTools.countMatches(key, "*") != 2 ) {
            throw new IllegalArgumentException( "StarDoublePathElement should have two '*' in its key. Was: " + key );
        }

        String[] split = key.split("\\*");
        boolean startsWithStar = key.startsWith( "*" );
        boolean endsWithStar = key.endsWith("*");
        if (  startsWithStar && endsWithStar) {
            prefix = "";
            mid = split[1];
            suffix = "";
        }
        else if ( endsWithStar ) {
            prefix = split[0];
            mid = split[1];
            suffix = "";
        }
        else if ( startsWithStar ) {
            prefix = "";
            mid = split[1];
            suffix = split[2];
        }
        else{
            prefix=split[0];
            mid=split[1];
            suffix=split[2];
        }
    }

It is found that the constructor of com.bazaarvoice.jolt.common.pathelement.StarDoublePathElement class could throw an unexpected ArrayIndexOutOfBoundsException if an invalid key string is provided. This happened because of a wrong assumption of the String::split(String) method used in the constructor. According to JDK Javadoc in https://docs.oracle.com/javase/8/docs/api/java/lang/String.html#split-java.lang.String-, the calling to the split method without providing the limit is equal to setting the limit to 0. When the limit is set to 0, all trailing empty strings are not included in the resulting array.
For example, calling "**".split("*"); results in an empty String array and calling "F**".split("*"); results in an array with a single element "F".

This wrong assumption of the string splitting makes the following access to the split array throw an unexpected ArrayIndexOutOfBoundsException.

For example, passing ** to the constructor passes the checking of double stars and both startsWithStar and endsWithStar will be true, but the String[] split will be an empty array and thus the call to split[1] later will throw an unexpected ArrayIndexOutOfBoundsException. Alternatively, passing F** will have a similar effect but it is triggered in different sections of the conditional branches since startsWithStar is false and endsWithStar is true for this string.

Proof of concept

Just compile any of the following Java code and run it could trigger the bug.

import com.bazaarvoice.jolt.common.pathelement.StarDoublePathElement;

public class ProofOfConcept {
  public static void main(String...args) {
    new StarDoublePathElement("**");
  }
}
import com.bazaarvoice.jolt.common.pathelement.StarDoublePathElement;

public class ProofOfConcept {
  public static void main(String...args) {
    new StarDoublePathElement("F**");
  }
}

Suggested fix

Instead of using String::split(String), use String::split(String, int) and provide a needed limit to ensure enough items exist in the resulting array. Alternatively, an additional check could be added to ensure a malformed string is denied.

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

No branches or pull requests

1 participant