From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 9468A45C304 for ; Tue, 1 Dec 2020 19:31:43 +0300 (MSK) From: Sergey Nikiforov Date: Tue, 1 Dec 2020 19:30:48 +0300 Message-Id: <20201201163048.95601-1-void@tarantool.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH] Fix base64 decoder output buffer overrun (reads) List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org Cc: Vladislav Shpilevoy It also caused data corruption. Also: Fixed read access beyond decode table (noticed along the way). Minimized number of condition checks in internal loops (performance). Fixes: #3069 --- third_party/base64.c | 57 ++++++++++++++++++++++++++++++-------------- 1 file changed, 39 insertions(+), 18 deletions(-) diff --git a/third_party/base64.c b/third_party/base64.c index 8ecab23eb..ab644df22 100644 --- a/third_party/base64.c +++ b/third_party/base64.c @@ -222,7 +222,8 @@ base64_decode_value(int value) 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51 }; - static const int decoding_size = sizeof(decoding); + static const int decoding_size = + sizeof(decoding) / sizeof(decoding[0]); int codepos = value; codepos -= 43; if (codepos < 0 || codepos >= decoding_size) @@ -247,8 +248,9 @@ base64_decode_block(const char *in_base64, int in_len, char *out_pos = out_bin; char *out_end = out_bin + out_len; int fragment; + char curr_byte; - *out_pos = state->result; + curr_byte = state->result; switch (state->step) { @@ -256,52 +258,71 @@ base64_decode_block(const char *in_base64, int in_len, { case step_a: do { - if (in_pos == in_end || out_pos >= out_end) + if (in_pos >= in_end) { state->step = step_a; - state->result = *out_pos; + state->result = curr_byte; return out_pos - out_bin; } fragment = base64_decode_value(*in_pos++); } while (fragment < 0); - *out_pos = (fragment & 0x03f) << 2; + curr_byte = (fragment & 0x03f) << 2; case step_b: do { - if (in_pos == in_end || out_pos >= out_end) + if (in_pos >= in_end) { state->step = step_b; - state->result = *out_pos; + state->result = curr_byte; return out_pos - out_bin; } fragment = base64_decode_value(*in_pos++); } while (fragment < 0); - *out_pos++ |= (fragment & 0x030) >> 4; - if (out_pos < out_end) - *out_pos = (fragment & 0x00f) << 4; + curr_byte |= (fragment & 0x030) >> 4; + *out_pos = curr_byte; + curr_byte = (fragment & 0x00f) << 4; + if (++out_pos >= out_end) + { + state->step = step_c; + state->result = curr_byte; + return out_pos - out_bin; + } case step_c: do { - if (in_pos == in_end || out_pos >= out_end) + if (in_pos >= in_end) { state->step = step_c; - state->result = *out_pos; + state->result = curr_byte; return out_pos - out_bin; } fragment = base64_decode_value(*in_pos++); } while (fragment < 0); - *out_pos++ |= (fragment & 0x03c) >> 2; - if (out_pos < out_end) - *out_pos = (fragment & 0x003) << 6; + curr_byte |= (fragment & 0x03c) >> 2; + *out_pos = curr_byte; + curr_byte = (fragment & 0x003) << 6; + if (++out_pos >= out_end) + { + state->step = step_d; + state->result = curr_byte; + return out_pos - out_bin; + } case step_d: do { - if (in_pos == in_end || out_pos >= out_end) + if (in_pos >= in_end) { state->step = step_d; - state->result = *out_pos; + state->result = curr_byte; return out_pos - out_bin; } fragment = base64_decode_value(*in_pos++); } while (fragment < 0); - *out_pos++ |= (fragment & 0x03f); + curr_byte |= (fragment & 0x03f); + *out_pos = curr_byte; + if (++out_pos >= out_end) + { + state->step = step_a; + state->result = curr_byte; + return out_pos - out_bin; + } } } /* control should not reach here */ -- 2.25.1