diff --git a/ChangeLog b/ChangeLog index 10f59071a..b7d7fd1eb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2015-06-01 Moritz Bunkus + + * mkvmerge, mkvpropedit: bug fix: fixed an invalid memory access + leading to a crash in the Base 64 decoder. Fixes #1222. + 2015-05-31 Moritz Bunkus * MKVToolNix GUI: bug fix: fixed progress parsing for interface diff --git a/src/common/base64.cpp b/src/common/base64.cpp index cc30b4c18..9c00140fc 100644 --- a/src/common/base64.cpp +++ b/src/common/base64.cpp @@ -96,12 +96,12 @@ base64_encode(const unsigned char *src, return out; } -int -base64_decode(const std::string &src, - unsigned char *dst) { - unsigned int pos = 0; - unsigned int dst_pos = 0; - unsigned int pad = 0; +std::string +base64_decode(std::string const &src) { + auto pos = 0u; + auto pad = 0u; + auto dst = std::string{}; + while (pos < src.size()) { unsigned char in[4]; unsigned int in_pos = 0; @@ -118,7 +118,7 @@ base64_decode(const std::string &src, pad++; else if (!isblanktab(c) && !iscr(c)) - return -1; + throw mtx::base64::invalid_data_x{}; } unsigned int values_idx; @@ -134,7 +134,7 @@ base64_decode(const std::string &src, : 255; if (255 == values[values_idx]) - throw mtx::base64::invalid_data_x(); + throw mtx::base64::invalid_data_x{}; } unsigned char mid[6]; @@ -145,20 +145,16 @@ base64_decode(const std::string &src, mid[4] = values[2] << 6; mid[5] = values[3]; - dst[dst_pos] = mid[0] | mid[1]; - dst_pos++; + dst += mid[0] | mid[1]; if (1 >= pad) { - dst[dst_pos] = mid[2] | mid[3]; - dst_pos++; - if (0 == pad) { - dst[dst_pos] = mid[4] | mid[5]; - dst_pos++; - } + dst += mid[2] | mid[3]; + if (0 == pad) + dst += mid[4] | mid[5]; } if (0 != pad) - return dst_pos; + break; } - return dst_pos; + return dst; } diff --git a/src/common/base64.h b/src/common/base64.h index 25175594a..034b7e171 100644 --- a/src/common/base64.h +++ b/src/common/base64.h @@ -34,6 +34,6 @@ namespace mtx { } std::string base64_encode(const unsigned char *src, int src_len, bool line_breaks = false, int max_line_len = 72); -int base64_decode(const std::string &src, unsigned char *dst); +std::string base64_decode(std::string const &src); #endif // MTX_COMMON_BASE64_H diff --git a/src/common/hacks.cpp b/src/common/hacks.cpp index 8843b8f4b..85e0a53b8 100644 --- a/src/common/hacks.cpp +++ b/src/common/hacks.cpp @@ -72,9 +72,7 @@ engage_hacks(const std::string &hacks) { "IE9oIGhvbmV5LCB0aGF0J3Mgc28gc3dlZXQhCiAgIC8tLS0tLS0tXC8gICBPZiB" "jb3Vyc2UgSSdsbCBtYXJyeSB5b3UhCiAgLyB8ICAgICB8fAogKiAgfHwtLS0tfH" "wKICAgIF5eICAgIF5eCg=="; - char correction[200]; - memset(correction, 0, 200); - base64_decode(initial, (unsigned char *)correction); + auto correction = base64_decode(initial); mxinfo(correction); mxexit(); } diff --git a/src/common/xml/ebml_converter.cpp b/src/common/xml/ebml_converter.cpp index 7f0e114dc..8c34d4189 100644 --- a/src/common/xml/ebml_converter.cpp +++ b/src/common/xml/ebml_converter.cpp @@ -275,11 +275,7 @@ ebml_converter_c::parse_binary(parser_context_t &ctx) { } else if (format == "base64") { try { - std::string destination(' ', content.size()); - auto dest_len = base64_decode(content, reinterpret_cast(&destination[0])); - if (-1 == dest_len) - throw malformed_data_x{ ctx.name, ctx.node.offset_debug(), Y("Invalid data for Base64 encoding found.") }; - content = destination.substr(0, dest_len); + content = base64_decode(content); } catch (mtx::base64::exception &) { throw malformed_data_x{ ctx.name, ctx.node.offset_debug(), Y("Invalid data for Base64 encoding found.") }; diff --git a/tests/results.txt b/tests/results.txt index 6eae5e4ed..f4a3eafa0 100644 --- a/tests/results.txt +++ b/tests/results.txt @@ -342,3 +342,4 @@ T_493truehd_ac3_setting_track_properties_mpeg_ts:8256eca144895021b5cf265efaf7bf2 T_494dont_abort_with_aac_error_proection_specific_config:f18d6c1a0cd91fcb216fac7ee68bd5f1:passed:20150416-092327:0.59008572 T_495default_durataion_and_sync:b360d52e56d8d291a7dc24ecd2b63c18-0a38dc656269f588fc4228d50e930d12-8f3d5eeacee5a77075eed0e7ae0d882a:passed:20150417-213623:0.894261521 T_496segment_size_0:9fd6d5e57b47693a483f9aad112a6a15-a78f5ddc1a9fb900b3797ced8752cb92:passed:20150530-180226:0.051304978 +T_497crash_in_base64_decoder:f6526cfaaef01627c52ee2ba25f03255:passed:20150601-192215:0.02848152 diff --git a/tests/test-497crash_in_base64_decoder.rb b/tests/test-497crash_in_base64_decoder.rb new file mode 100755 index 000000000..3b51ba9fd --- /dev/null +++ b/tests/test-497crash_in_base64_decoder.rb @@ -0,0 +1,5 @@ +#!/usr/bin/ruby -w + +# T_497crash_in_base64_decoder +describe "mkvmerge / crash in the Base 64 decoder" +test_merge "--tags 0:data/text/tags-crash-invalid-memory-access.xml data/subtitles/srt/ven.srt"