From fd9744968f7112b85a0b3ffab2080f1d12bb9823 Mon Sep 17 00:00:00 2001 From: Alexander Reelsen Date: Tue, 24 Jun 2014 13:01:11 +0200 Subject: [PATCH] Internal: Made base64 decode parsing to detect more errors The base64 did not completely check, if there were other characters after the equals `=` sign. This PR adds some small additional checks. Closes #6334 --- .../java/org/elasticsearch/common/Base64.java | 11 ++++ .../org/elasticsearch/common/Base64Test.java | 59 +++++++++++++++++++ 2 files changed, 70 insertions(+) create mode 100644 src/test/java/org/elasticsearch/common/Base64Test.java diff --git a/src/main/java/org/elasticsearch/common/Base64.java b/src/main/java/org/elasticsearch/common/Base64.java index 7860279085c..03cc2a46b6d 100644 --- a/src/main/java/org/elasticsearch/common/Base64.java +++ b/src/main/java/org/elasticsearch/common/Base64.java @@ -1217,9 +1217,20 @@ public class Base64 { // If that was the equals sign, break out of 'for' loop if (source[i] == EQUALS_SIGN) { + // check if the equals sign is somewhere in between + if (i+1 < len + off) { + throw new java.io.IOException(String.format(Locale.ROOT, + "Found equals sign at position %d of the base64 string, not at the end", i)); + } break; } // end if: equals sign } // end if: quartet built + else { + if (source[i] == EQUALS_SIGN && len + off > i && source[i+1] != EQUALS_SIGN) { + throw new java.io.IOException(String.format(Locale.ROOT, + "Found equals sign at position %d of the base64 string, not at the end", i)); + } // enf if: equals sign and next character not as well + } // end else: } // end if: equals sign or better } // end if: white space, equals sign or better else { diff --git a/src/test/java/org/elasticsearch/common/Base64Test.java b/src/test/java/org/elasticsearch/common/Base64Test.java new file mode 100644 index 00000000000..f80d6afb445 --- /dev/null +++ b/src/test/java/org/elasticsearch/common/Base64Test.java @@ -0,0 +1,59 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.elasticsearch.common; + +import com.google.common.base.Charsets; +import org.elasticsearch.test.ElasticsearchTestCase; +import org.junit.Test; + +import java.io.IOException; +import java.util.Locale; + +import static org.hamcrest.Matchers.is; + +/** + * + */ +public class Base64Test extends ElasticsearchTestCase { + + @Test // issue #6334 + public void testBase64DecodeWithExtraCharactersAfterPadding() throws Exception { + String plain = randomAsciiOfLengthBetween(1, 20) + ":" + randomAsciiOfLengthBetween(1, 20); + String encoded = Base64.encodeBytes(plain.getBytes(Charsets.UTF_8)); + assertValidBase64(encoded, plain); + + // lets append some trash here, if the encoded string has been padded + char lastChar = encoded.charAt(encoded.length() - 1); + if (lastChar == '=') { + assertInvalidBase64(encoded + randomAsciiOfLength(3)); + } + } + + private void assertValidBase64(String base64, String expected) throws IOException { + String decoded = new String(Base64.decode(base64.getBytes(Charsets.UTF_8)), Charsets.UTF_8); + assertThat(decoded, is(expected)); + } + + private void assertInvalidBase64(String base64) { + try { + Base64.decode(base64.getBytes(Charsets.UTF_8)); + fail(String.format(Locale.ROOT, "Expected IOException to be thrown for string %s (len %d)", base64, base64.length())); + } catch (IOException e) {} + } +}