From afa3db0a6f15e1b1038cb47f0632baa8f23d0f67 Mon Sep 17 00:00:00 2001 From: Stephan Bosch Date: Mon, 26 Aug 2019 13:09:30 +0200 Subject: [PATCH] lib: base64 - Deprecate src_pos_r parameter of base64_decode(). Only NULL pointer is allowed. This allows using the new incremental API internally, thereby dropping the old decoder implementation. --- src/doveadm/dsync/dsync-mailbox-state.c | 3 +- src/lib-mail/istream-attachment-extractor.c | 9 ++- src/lib-mail/message-decoder.c | 16 +++-- src/lib/base64.c | 72 +++------------------ src/lib/base64.h | 27 ++++---- src/lib/test-base64.c | 46 +++++-------- 6 files changed, 57 insertions(+), 116 deletions(-) diff --git a/src/doveadm/dsync/dsync-mailbox-state.c b/src/doveadm/dsync/dsync-mailbox-state.c index b77b116078..cca24d4bc5 100644 --- a/src/doveadm/dsync/dsync-mailbox-state.c +++ b/src/doveadm/dsync/dsync-mailbox-state.c @@ -81,11 +81,10 @@ int dsync_mailbox_states_import(HASH_TABLE_TYPE(dsync_mailbox_state) states, buffer_t *buf; uint8_t *guid_p; const unsigned char *data; - size_t pos; unsigned int i, count; buf = t_buffer_create(strlen(input)); - if (base64_decode(input, strlen(input), &pos, buf) < 0) { + if (base64_decode(input, strlen(input), NULL, buf) < 0) { *error_r = "Invalid base64 data"; return -1; } diff --git a/src/lib-mail/istream-attachment-extractor.c b/src/lib-mail/istream-attachment-extractor.c index af089582eb..e9655a5a67 100644 --- a/src/lib-mail/istream-attachment-extractor.c +++ b/src/lib-mail/istream-attachment-extractor.c @@ -341,6 +341,7 @@ static int astream_decode_base64(struct attachment_istream *astream, buffer_t **extra_buf_r) { struct attachment_istream_part *part = &astream->part; + struct base64_decoder b64dec; struct istream *input, *base64_input; struct ostream *output; const unsigned char *data; @@ -376,12 +377,13 @@ static int astream_decode_base64(struct attachment_istream *astream, output = o_stream_create_fd_file(outfd, 0, FALSE); o_stream_cork(output); + base64_decode_init(&b64dec, &base64_scheme, 0); hash_format_reset(astream->set.hash_format); size_t bytes_needed = 1; while ((ret = i_stream_read_bytes(base64_input, &data, &size, bytes_needed)) > 0) { buffer_set_used_size(buf, 0); - if (base64_decode(data, size, &size, buf) < 0) { + if (base64_decode_more(&b64dec, data, size, &size, buf) < 0) { i_error("istream-attachment: BUG: " "Attachment base64 data unexpectedly broke"); failed = TRUE; @@ -401,6 +403,11 @@ static int astream_decode_base64(struct attachment_istream *astream, i_stream_get_error(base64_input)); failed = TRUE; } + if (base64_decode_finish(&b64dec) < 0) { + i_error("istream-attachment: BUG: " + "Attachment base64 data unexpectedly broke"); + failed = TRUE; + } if (o_stream_finish(output) < 0) { i_error("istream-attachment: write(%s) failed: %s", o_stream_get_name(output), o_stream_get_error(output)); diff --git a/src/lib-mail/message-decoder.c b/src/lib-mail/message-decoder.c index d5ac9f8a58..83eff840e8 100644 --- a/src/lib-mail/message-decoder.c +++ b/src/lib-mail/message-decoder.c @@ -273,6 +273,7 @@ static bool message_decode_body(struct message_decoder_context *ctx, struct message_block *input, struct message_block *output) { + struct base64_decoder b64dec; const unsigned char *data = NULL; size_t pos = 0, size = 0; const char *error; @@ -307,15 +308,18 @@ static bool message_decode_body(struct message_decoder_context *ctx, } case MESSAGE_CTE_BASE64: buffer_set_used_size(ctx->buf, 0); + base64_decode_init(&b64dec, &base64_scheme, 0); if (ctx->encoding_buf->used != 0) { - ret = base64_decode(ctx->encoding_buf->data, - ctx->encoding_buf->used, - &pos, ctx->buf); + ret = base64_decode_more(&b64dec, + ctx->encoding_buf->data, + ctx->encoding_buf->used, + &pos, ctx->buf); } else { - ret = base64_decode(input->data, input->size, - &pos, ctx->buf); + ret = base64_decode_more(&b64dec, + input->data, input->size, + &pos, ctx->buf); } - if (ret < 0) { + if (ret < 0 || base64_decode_finish(&b64dec) < 0) { /* corrupted base64 data, don't bother with the rest of it */ return FALSE; diff --git a/src/lib/base64.c b/src/lib/base64.c index fe0c62ee98..42bf5322f6 100644 --- a/src/lib/base64.c +++ b/src/lib/base64.c @@ -524,71 +524,15 @@ int base64_decode_finish(struct base64_decoder *dec) */ int base64_scheme_decode(const struct base64_scheme *b64, - const void *src, size_t src_size, size_t *src_pos_r, - buffer_t *dest) + const void *src, size_t src_size, buffer_t *dest) { - const unsigned char *b64dec = b64->decmap; - const unsigned char *src_c = src; - size_t src_pos; - unsigned char input[4], output[3]; - int ret = 1; - - for (src_pos = 0; src_pos+3 < src_size; ) { - input[0] = b64dec[src_c[src_pos]]; - if (input[0] == 0xff) { - if (unlikely(!IS_EMPTY(src_c[src_pos]))) { - ret = -1; - break; - } - src_pos++; - continue; - } - - input[1] = b64dec[src_c[src_pos+1]]; - if (unlikely(input[1] == 0xff)) { - ret = -1; - break; - } - output[0] = (input[0] << 2) | (input[1] >> 4); + struct base64_decoder dec; + int ret; - input[2] = b64dec[src_c[src_pos+2]]; - if (input[2] == 0xff) { - if (unlikely(src_c[src_pos+2] != '=' || - src_c[src_pos+3] != '=')) { - ret = -1; - break; - } - buffer_append(dest, output, 1); - ret = 0; - src_pos += 4; - break; - } - - output[1] = (input[1] << 4) | (input[2] >> 2); - input[3] = b64dec[src_c[src_pos+3]]; - if (input[3] == 0xff) { - if (unlikely(src_c[src_pos+3] != '=')) { - ret = -1; - break; - } - buffer_append(dest, output, 2); - ret = 0; - src_pos += 4; - break; - } - - output[2] = ((input[2] << 6) & 0xc0) | input[3]; - buffer_append(dest, output, 3); - src_pos += 4; - } - - for (; src_pos < src_size; src_pos++) { - if (!IS_EMPTY(src_c[src_pos])) - break; - } - - if (src_pos_r != NULL) - *src_pos_r = src_pos; + base64_decode_init(&dec, b64, 0); + ret = base64_decode_more(&dec, src, src_size, NULL, dest); + if (ret >= 0) + ret = base64_decode_finish(&dec); return ret; } @@ -600,7 +544,7 @@ buffer_t *t_base64_scheme_decode_str(const struct base64_scheme *b64, size_t len = strlen(str); buf = t_buffer_create(MAX_BASE64_DECODED_SIZE(len)); - (void)base64_scheme_decode(b64, str, len, NULL, buf); + (void)base64_scheme_decode(b64, str, len, buf); return buf; } diff --git a/src/lib/base64.h b/src/lib/base64.h index ed455f6191..7121a38824 100644 --- a/src/lib/base64.h +++ b/src/lib/base64.h @@ -189,13 +189,9 @@ base64_scheme_encode(const struct base64_scheme *b64, Any CR, LF characters are ignored, as well as whitespace at beginning or end of line. - - This function may be called multiple times for parsing the same stream. - If src_pos is non-NULL, it's updated to first non-translated character in - src. */ + */ int base64_scheme_decode(const struct base64_scheme *b64, - const void *src, size_t src_size, size_t *src_pos_r, - buffer_t *dest) ATTR_NULL(4); + const void *src, size_t src_size, buffer_t *dest); /* Decode given string to a buffer allocated from data stack. @@ -227,13 +223,18 @@ base64_encode(const void *src, size_t src_size, buffer_t *dest) } /* Translates base64 data into binary and appends it to dest buffer. See - base64_scheme_decode(). */ + base64_scheme_decode(). + + The src_pos_r parameter is deprecated and MUST be NULL. + */ static inline int -base64_decode(const void *src, size_t src_size, size_t *src_pos_r, +base64_decode(const void *src, size_t src_size, size_t *src_pos_r ATTR_UNUSED, buffer_t *dest) ATTR_NULL(3) { - return base64_scheme_decode(&base64_scheme, src, src_size, - src_pos_r, dest); + // NOTE: src_pos_r is deprecated here; to be removed in v2.4 */ + i_assert(src_pos_r == NULL); + + return base64_scheme_decode(&base64_scheme, src, src_size, dest); } /* Decode given string to a buffer allocated from data stack. */ @@ -264,11 +265,9 @@ base64url_encode(const void *src, size_t src_size, buffer_t *dest) /* Translates base64url data into binary and appends it to dest buffer. See base64_scheme_decode(). */ static inline int -base64url_decode(const void *src, size_t src_size, size_t *src_pos_r, - buffer_t *dest) ATTR_NULL(3) +base64url_decode(const void *src, size_t src_size, buffer_t *dest) { - return base64_scheme_decode(&base64url_scheme, src, src_size, - src_pos_r, dest); + return base64_scheme_decode(&base64url_scheme, src, src_size, dest); } /* Decode given string to a buffer allocated from data stack. */ diff --git a/src/lib/test-base64.c b/src/lib/test-base64.c index 38873816c2..79702de5d0 100644 --- a/src/lib/test-base64.c +++ b/src/lib/test-base64.c @@ -40,35 +40,33 @@ struct test_base64_decode { const char *input; const char *output; int ret; - unsigned int src_pos; }; static void test_base64_decode(void) { static const struct test_base64_decode tests[] = { { "\taGVsbG8gd29ybGQ=", - "hello world", 0, UINT_MAX }, + "hello world", 0 }, { "\nZm9v\n \tIGJh \t\ncml0cw==", - "foo barits", 0, UINT_MAX }, + "foo barits", 0 }, { " anVzdCBuaWlu \n", - "just niin", 1, UINT_MAX }, + "just niin", 0 }, { "aGVsb", - "hel", 1, 4 }, + "hel", -1 }, { "aGVsb!!!!!", - "hel", -1, 4 }, + "hel", -1 }, { "aGVs!!!!!", - "hel", -1, 4 }, + "hel", -1 }, { "0JPQvtCy0L7RgNGPzIHRgiwg0YfRgt" "C+INC60YPRgCDQtNC+0Y/MgdGCLg==", "\xd0\x93\xd0\xbe\xd0\xb2\xd0\xbe\xd1\x80\xd1\x8f\xcc" "\x81\xd1\x82\x2c\x20\xd1\x87\xd1\x82\xd0\xbe\x20\xd0" "\xba\xd1\x83\xd1\x80\x20\xd0\xb4\xd0\xbe\xd1\x8f\xcc" - "\x81\xd1\x82\x2e", 0, UINT_MAX }, + "\x81\xd1\x82\x2e", 0 }, }; string_t *str; buffer_t buf; unsigned int i; - size_t src_pos; int ret; test_begin("base64_decode()"); @@ -83,17 +81,13 @@ static void test_base64_decode(void) buffer_create_from_data(&buf, t_malloc0(max_decoded_size), max_decoded_size); str = &buf; - src_pos = 0; ret = base64_decode(tests[i].input, strlen(tests[i].input), - &src_pos, str); + NULL, str); test_assert_idx(tests[i].ret == ret, i); test_assert_idx(strlen(tests[i].output) == str_len(str) && memcmp(tests[i].output, str_data(str), str_len(str)) == 0, i); - test_assert_idx(src_pos == tests[i].src_pos || - (tests[i].src_pos == UINT_MAX && - src_pos == strlen(tests[i].input)), i); if (ret >= 0) { test_assert_idx( str_len(str) <= MAX_BASE64_DECODED_SIZE( @@ -165,35 +159,33 @@ struct test_base64url_decode { const char *input; const char *output; int ret; - unsigned int src_pos; }; static void test_base64url_decode(void) { static const struct test_base64url_decode tests[] = { { "\taGVsbG8gd29ybGQ=", - "hello world", 0, UINT_MAX }, + "hello world", 0 }, { "\nZm9v\n \tIGJh \t\ncml0cw==", - "foo barits", 0, UINT_MAX }, + "foo barits", 0 }, { " anVzdCBuaWlu \n", - "just niin", 1, UINT_MAX }, + "just niin", 0 }, { "aGVsb", - "hel", 1, 4 }, + "hel", -1 }, { "aGVsb!!!!!", - "hel", -1, 4 }, + "hel", -1 }, { "aGVs!!!!!", - "hel", -1, 4 }, + "hel", -1 }, { "0JPQvtCy0L7RgNGPzIHRgiwg0YfRgt" "C-INC60YPRgCDQtNC-0Y_MgdGCLg==", "\xd0\x93\xd0\xbe\xd0\xb2\xd0\xbe\xd1\x80\xd1\x8f\xcc" "\x81\xd1\x82\x2c\x20\xd1\x87\xd1\x82\xd0\xbe\x20\xd0" "\xba\xd1\x83\xd1\x80\x20\xd0\xb4\xd0\xbe\xd1\x8f\xcc" - "\x81\xd1\x82\x2e", 0, UINT_MAX }, + "\x81\xd1\x82\x2e", 0 }, }; string_t *str; buffer_t buf; unsigned int i; - size_t src_pos; int ret; test_begin("base64url_decode()"); @@ -208,17 +200,13 @@ static void test_base64url_decode(void) buffer_create_from_data(&buf, t_malloc0(max_decoded_size), max_decoded_size); str = &buf; - src_pos = 0; ret = base64url_decode(tests[i].input, strlen(tests[i].input), - &src_pos, str); + str); test_assert_idx(tests[i].ret == ret, i); test_assert_idx(strlen(tests[i].output) == str_len(str) && memcmp(tests[i].output, str_data(str), str_len(str)) == 0, i); - test_assert_idx(src_pos == tests[i].src_pos || - (tests[i].src_pos == UINT_MAX && - src_pos == strlen(tests[i].input)), i); if (ret >= 0) { test_assert_idx( str_len(str) <= MAX_BASE64_DECODED_SIZE( @@ -247,7 +235,7 @@ static void test_base64url_random(void) str_truncate(dest, 0); base64url_encode(buf, max, str); test_assert_idx(base64url_decode(str_data(str), str_len(str), - NULL, dest) >= 0, i); + dest) >= 0, i); test_assert_idx(str_len(dest) == max && memcmp(buf, str_data(dest), max) == 0, i); } -- 2.47.3