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

Internal: Base64 decode parsing detects more errors #6334

Closed
erikringsmuth opened this issue May 28, 2014 · 0 comments · Fixed by #6348
Closed

Internal: Base64 decode parsing detects more errors #6334

erikringsmuth opened this issue May 28, 2014 · 0 comments · Fixed by #6348

Comments

@erikringsmuth
Copy link
Contributor

I found a bug in the Base64.decode() function. It will decode incorrectly formatted base 64 strings. For example, "user:password" encodes to "dXNlcjpwYXNzd29yZA==". Both "dXNlcjpwYXNzd29yZA==123" and "dXNlcjpwYXNzd29yZA=5" decode to "user:password" although neither of them are valid base 64 strings. You can add or replace characters after the first padding character.

I wrote a quick test to show the bug.

@Test
public void testBase64DecodeWithExtraCharactersAfterPadding() {
    // "user:password" encodes to "dXNlcjpwYXNzd29yZA=="
    String correctlyEncoded = "";
    String extraCharacters = "";
    String replacePaddingCharacter = "";
    try {
        correctlyEncoded = new String(Base64.decode("dXNlcjpwYXNzd29yZA=="));
        extraCharacters = new String(Base64.decode("dXNlcjpwYXNzd29yZA==123"));
        replacePaddingCharacter = new String(Base64.decode("dXNlcjpwYXNzd29yZA=5"));
    } catch (IOException e) {
    }
    assertEquals("user:password", correctlyEncoded);
    assertNotEquals("user:password", extraCharacters); // this assertion fails
    assertNotEquals("user:password", replacePaddingCharacter); // this assertion fails
}

I think this line is the culprit.
https://github.com/elasticsearch/elasticsearch/blob/master/src/main/java/org/elasticsearch/common/Base64.java#L1219

It breaks early when it should really throw an IOException like the code 8 lines down.

spinscale added a commit to spinscale/elasticsearch that referenced this issue Jun 24, 2014
The base64 did not completely check, if there were other characters
after the equals `=` sign. This PR adds some small additional checks.

Closes elastic#6334
spinscale added a commit that referenced this issue Jun 24, 2014
The base64 did not completely check, if there were other characters
after the equals `=` sign. This PR adds some small additional checks.

Closes #6334
@spinscale spinscale changed the title Base64.decode() decoding invalid strings Internal: Made base64 decode parsing to detect more errors Jun 24, 2014
@spinscale spinscale changed the title Internal: Made base64 decode parsing to detect more errors Internal: Base64 decode parsing detects more errors Jun 24, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants