From: Jacob Champion Date: Tue, 20 Jun 2017 23:55:20 +0000 (+0000) Subject: util.c: add a strict Base64 decoding function X-Git-Tag: 2.5.0-alpha~365 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e0852fd12e2ac4afc7540eb78840c4a11cfa2e29;p=apache util.c: add a strict Base64 decoding function ap_pbase64decode_strict() adds to the functionality of ap_pbase64decode() in two ways: - the length of the decoded buffer is returned, allowing embedded NULLs to be retained by the caller - the input string is strictly checked for Base64 validity, including correct zero-padding at the end of the string (This was originally added to the httpdunit feature/backport branch in r1796208, then reverted in r1799376, since it's currently intended for trunk only.) git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1799380 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/include/httpd.h b/include/httpd.h index 1a33466aeb..1a23333959 100644 --- a/include/httpd.h +++ b/include/httpd.h @@ -1997,6 +1997,28 @@ AP_DECLARE(const char *) ap_stripprefix(const char *bigstring, */ AP_DECLARE(char *) ap_pbase64decode(apr_pool_t *p, const char *bufcoded); +/** + * Decode a base64 encoded string into memory allocated from a pool, while + * ensuring that the input string is in fact valid base64. + * + * Unlike ap_pbase64decode(), this function allows encoded NULLs in the input to + * be retained by the caller, by inspecting the len argument after the call + * instead of using strlen(). A NULL terminator is still appended to the buffer + * to faciliate string use (it is not included in len). + * + * @param p The pool to allocate from + * @param encoded The encoded string + * @param decoded On success, set to the decoded buffer, which is allocated from + * p + * @param len On success, set to the length of the decoded buffer (not including + * the terminating NULL byte) + * @return APR_SUCCESS if the decoding was successful + */ +AP_DECLARE(apr_status_t) ap_pbase64decode_strict(apr_pool_t *p, + const char *encoded, + char **decoded, + apr_size_t *len); + /** * Encode a string into memory allocated from a pool in base 64 format * @param p The pool to allocate from diff --git a/server/util.c b/server/util.c index 3c001511be..5d1df08ae3 100644 --- a/server/util.c +++ b/server/util.c @@ -2386,6 +2386,76 @@ AP_DECLARE(char *) ap_pbase64decode(apr_pool_t *p, const char *bufcoded) return decoded; } +/* a stringent version of ap_pbase64decode() */ +AP_DECLARE(apr_status_t) ap_pbase64decode_strict(apr_pool_t *p, + const char *encoded, + char **decoded, + apr_size_t *len) +{ + apr_size_t end_index; + int last_group_len; + const char *end; + + /* Sanity check. + * TODO: this would be a lot more efficient if we had access to the lookup + * table used by APR. If that gets pulled in at any point, make use of it. + */ + end_index = strspn(encoded, "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "abcdefghijklmnopqrstuvwxyz" + "0123456789+/"); + + last_group_len = end_index % 4; + end = encoded + end_index; + + /* The only non-alphabet character allowed is the padding character '=' at + * the end of the string. There are two allowable padding cases for the last + * group of four: "xY==" or "xyZ=". We require the final (non-padding) + * character to have been zero-padded during encoding, which limits the + * character choices. + */ + if (last_group_len == 1) { + /* This isn't ever valid. */ + return APR_EINVAL; + } + else if (last_group_len == 2) { + /* ...xY== */ + if (*end != '=' || end[1] != '=') { + return APR_EINVAL; + } + else if (!ap_strchr("AQgw", end[-1])) { + /* Correctly zero-padded input data will result in a final character + * that is one of the four above. */ + return APR_EINVAL; + } + + end += 2; + } + else if (last_group_len == 3) { + /* ...xyZ= */ + if (*end != '=') { + return APR_EINVAL; + } + else if (!ap_strchr("AEIMQUYcgkosw048", end[-1])) { + /* Correctly zero-padded input data will result in a final character + * that is one of the sixteen above. */ + return APR_EINVAL; + } + + end++; + } + + /* At this point, if the encoded buffer is correct, we should be at the end + * of the string. */ + if (*end) { + return APR_EINVAL; + } + + *decoded = apr_palloc(p, apr_base64_decode_len(encoded)); + *len = apr_base64_decode(*decoded, encoded); + + return APR_SUCCESS; +} + AP_DECLARE(char *) ap_pbase64encode(apr_pool_t *p, char *string) { char *encoded; diff --git a/test/unit/base64.c b/test/unit/base64.c new file mode 100644 index 0000000000..3e5b4f5015 --- /dev/null +++ b/test/unit/base64.c @@ -0,0 +1,175 @@ +/* Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF 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. + */ + +#include "../httpdunit.h" + +#include "httpd.h" + +/* + * Test Fixture -- runs once per test + */ + +static apr_pool_t *g_pool; + +static void base64_setup(void) +{ + if (apr_pool_create(&g_pool, NULL) != APR_SUCCESS) { + exit(1); + } +} + +static void base64_teardown(void) +{ + apr_pool_destroy(g_pool); +} + +/* + * Tests + */ + +/* + * case[0]: encoded value + * case[1]: expected decoded value + */ +static const char * const base64_cases[][2] = { + /* Test case 1 derived from RFC 3548 sec. 7. */ + { "FPucA9l+", "\x14\xfb\x9c\x03\xd9\x7e" }, + { "aGVsbG8=", "hello" }, + { "dGVzdA==", "test" }, + { "", "" }, +}; +static const size_t base64_cases_len = sizeof(base64_cases) / + sizeof(base64_cases[0]); + +HTTPD_START_LOOP_TEST(strict_decoding_works, base64_cases_len) +{ + const char *encoded = base64_cases[_i][0]; + const char *expected = base64_cases[_i][1]; + char *decoded; + apr_size_t len; + apr_status_t status; + + status = ap_pbase64decode_strict(g_pool, encoded, &decoded, &len); + + ck_assert_int_eq(status, APR_SUCCESS); + ck_assert_uint_eq(len, strlen(expected)); + ck_assert(!memcmp(decoded, expected, len)); +} +END_TEST + +START_TEST(strict_decoding_works_with_embedded_nulls) +{ + static unsigned char expected[] = { 'n', '\0', 'u', '\0', 'l', '\0', 'l' }; + char *decoded; + apr_size_t len; + apr_status_t status; + + status = ap_pbase64decode_strict(g_pool, "bgB1AGwAbA==", &decoded, &len); + + ck_assert_int_eq(status, APR_SUCCESS); + ck_assert_uint_eq(len, sizeof(expected)); + ck_assert(!memcmp(decoded, expected, len)); +} +END_TEST + +START_TEST(strict_decoding_produces_null_terminated_buffer) +{ + char *decoded; + apr_size_t len; + + ap_pbase64decode_strict(g_pool, "aaaabbbbccccdddd", &decoded, &len); + + ck_assert(decoded[len] == '\0'); +} +END_TEST + +START_TEST(strict_decoding_allows_all_base64_characters) +{ + char *decoded; + apr_size_t len; + apr_status_t status; + + status = ap_pbase64decode_strict(g_pool, + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "abcdefghijklmnopqrstuvwxyz" + "0123456789+/", + &decoded, &len);; + + ck_assert_int_eq(status, APR_SUCCESS); +} +END_TEST + +static const char * const invalid_chars[] = { + "bad?", + "not-good", + "also_bad", + "a\x01sd", +}; +static const size_t invalid_chars_len = sizeof(invalid_chars) / + sizeof(invalid_chars[0]); + +HTTPD_START_LOOP_TEST(strict_decoding_rejects_non_base64_characters, invalid_chars_len) +{ + char *decoded = NULL; + apr_size_t len = (apr_size_t) -1; + apr_status_t status; + + status = ap_pbase64decode_strict(g_pool, invalid_chars[_i], &decoded, &len); + + ck_assert_int_eq(status, APR_EINVAL); + ck_assert(decoded == NULL); + ck_assert_uint_eq(len, (apr_size_t) -1); +} +END_TEST + +static const char * const invalid_padding[] = { + "AAAA=", /* no padding needed here */ + "AA=", /* not enough padding */ + "AA===", /* too much padding */ + "A===", /* only one or two padding characters allowed */ + "A==", + "==", + "AAA=AAA=", /* mid-string padding prohibited */ + "AAA", /* missing padding entirely */ + "AA", /* missing padding entirely */ + "A", /* just completely wrong */ + "AAb=", /* one-padded strings must end in one of AEIMQUYcgkosw048 */ + "Ab==", /* two-padded strings must end in one of AQgw */ +}; +static const size_t invalid_padding_len = sizeof(invalid_padding) / + sizeof(invalid_padding[0]); + +HTTPD_START_LOOP_TEST(strict_decoding_rejects_incorrect_padding, invalid_padding_len) +{ + char *decoded = NULL; + apr_size_t len = (apr_size_t) -1; + apr_status_t status; + + status = ap_pbase64decode_strict(g_pool, invalid_padding[_i], &decoded, + &len); + + ck_assert_int_eq(status, APR_EINVAL); + ck_assert(decoded == NULL); + ck_assert_uint_eq(len, (apr_size_t) -1); +} +END_TEST + +/* + * Test Case Boilerplate + */ +HTTPD_BEGIN_TEST_CASE_WITH_FIXTURE(base64, base64_setup, base64_teardown) +#include "test/unit/base64.tests" +HTTPD_END_TEST_CASE