From 46c3a44b413b3abb6c7072d9d0a51a2a7646c3af Mon Sep 17 00:00:00 2001 From: Martin Michelsen Date: Sat, 25 Nov 2023 16:12:11 -0800 Subject: [PATCH] fix uninitialized memory bug in UTF16 encoder --- src/ProxyCommands.cc | 10 +++--- src/Text.hh | 80 +++++++++++++++++++++----------------------- 2 files changed, 44 insertions(+), 46 deletions(-) diff --git a/src/ProxyCommands.cc b/src/ProxyCommands.cc index 79c97b40..5687e78b 100644 --- a/src/ProxyCommands.cc +++ b/src/ProxyCommands.cc @@ -274,7 +274,7 @@ static HandlerResult S_V123P_02_17( C_LoginV1_DC_PC_V3_90 cmd; cmd.serial_number.encode(string_printf("%08" PRIX32 "", ses->license->serial_number)); cmd.access_key.encode(ses->license->access_key); - cmd.access_key.clear_after(8); + cmd.access_key.clear_after_bytes(8); ses->server_channel.send(0x90, 0x00, &cmd, sizeof(cmd)); return HandlerResult::Type::SUPPRESS; } else { @@ -293,7 +293,7 @@ static HandlerResult S_V123P_02_17( cmd.language = ses->language(); cmd.serial_number.encode(string_printf("%08" PRIX32 "", ses->license->serial_number)); cmd.access_key.encode(ses->license->access_key); - cmd.access_key.clear_after(8); + cmd.access_key.clear_after_bytes(8); cmd.hardware_id.encode(ses->hardware_id); cmd.name.encode(ses->character_name); ses->server_channel.send(0x93, 0x00, &cmd, sizeof(cmd)); @@ -316,7 +316,7 @@ static HandlerResult S_V123P_02_17( cmd.sub_version = ses->sub_version; cmd.serial_number.encode(string_printf("%08" PRIX32 "", ses->license->serial_number)); cmd.access_key.encode(ses->license->access_key); - cmd.access_key.clear_after(8); + cmd.access_key.clear_after_bytes(8); cmd.serial_number2 = cmd.serial_number; cmd.access_key2 = cmd.access_key; // TODO: We probably should set email_address, but we currently don't @@ -340,7 +340,7 @@ static HandlerResult S_V123P_02_17( cmd.language = ses->language(); cmd.serial_number.encode(string_printf("%08" PRIX32 "", ses->license->serial_number)); cmd.access_key.encode(ses->license->access_key); - cmd.access_key.clear_after(8); + cmd.access_key.clear_after_bytes(8); cmd.serial_number2 = cmd.serial_number; cmd.access_key2 = cmd.access_key; if (ses->config.check_flag(Client::Flag::PROXY_BLANK_NAME_ENABLED)) { @@ -1651,7 +1651,7 @@ static HandlerResult C_81(shared_ptr ses, uint16_t, } } // GC clients send uninitialized memory here; don't forward it - cmd.text.clear_after(cmd.text.used_chars_8()); + cmd.text.clear_after_bytes(cmd.text.used_chars_8()); return HandlerResult::Type::MODIFIED; } diff --git a/src/Text.hh b/src/Text.hh index c0502807..53cce3ef 100644 --- a/src/Text.hh +++ b/src/Text.hh @@ -308,7 +308,7 @@ struct pstring { pstring& operator=(const pstring& other) { size_t end_offset = std::min(Bytes, pstring::Bytes); memcpy(this->data, other.data, end_offset); - this->clear_after(end_offset); + this->clear_after_bytes(end_offset); return *this; } pstring& operator=(pstring&& s) = delete; @@ -318,54 +318,36 @@ struct pstring { switch (Encoding) { case TextEncoding::CHALLENGE8: case TextEncoding::ASCII: { - fprintf(stderr, "pstring encode ASCII/CHALLENGE8\n"); - print_data(stderr, s); auto ret = tt_utf8_to_ascii(this->data, Bytes, s.data(), s.size(), true); - this->clear_after(ret.bytes_written); + this->clear_after_bytes(ret.bytes_written); if (Encoding == TextEncoding::CHALLENGE8) { encrypt_challenge_rank_text_t(this->data, Bytes); } - print_data(stderr, this->data, Bytes); break; } case TextEncoding::ISO8859: { - fprintf(stderr, "pstring encode ISO8859\n"); - print_data(stderr, s); auto ret = tt_utf8_to_8859(this->data, Bytes, s.data(), s.size(), true); - this->clear_after(ret.bytes_written); - print_data(stderr, this->data, Bytes); + this->clear_after_bytes(ret.bytes_written); break; } case TextEncoding::SJIS: { - fprintf(stderr, "pstring encode SJIS\n"); - print_data(stderr, s); auto ret = tt_utf8_to_sjis(this->data, Bytes, s.data(), s.size(), true); - this->clear_after(ret.bytes_written); - print_data(stderr, this->data, Bytes); + this->clear_after_bytes(ret.bytes_written); break; } case TextEncoding::UTF16: { - fprintf(stderr, "pstring encode UTF16\n"); - print_data(stderr, s); auto ret = tt_utf8_to_utf16(this->data, Bytes, s.data(), s.size(), true); - this->clear_after(ret.bytes_written); - print_data(stderr, this->data, Bytes); + this->clear_after_bytes(ret.bytes_written); break; } case TextEncoding::UTF8: - fprintf(stderr, "pstring encode UTF8\n"); - print_data(stderr, s); memcpy(this->data, s.data(), std::min(s.size(), Bytes)); - this->clear_after(s.size()); - print_data(stderr, this->data, Bytes); + this->clear_after_bytes(s.size()); break; case TextEncoding::CHALLENGE16: { - fprintf(stderr, "pstring encode CHALLENGE16\n"); - print_data(stderr, s); auto ret = tt_utf8_to_utf16(this->data, Bytes, s.data(), s.size(), true); encrypt_challenge_rank_text_t(this->data, ret.bytes_written / 2); - this->clear_after(ret.bytes_written); - print_data(stderr, this->data, Bytes); + this->clear_after_bytes(ret.bytes_written); break; } case TextEncoding::MARKED: { @@ -374,22 +356,22 @@ struct pstring { if (client_language == 0) { try { auto ret = tt_utf8_to_sjis(this->data, Bytes, s.data(), s.size(), true); - this->clear_after(ret.bytes_written); + this->clear_after_bytes(ret.bytes_written); } catch (const std::runtime_error&) { this->data[0] = '\t'; this->data[1] = 'E'; auto ret = tt_utf8_to_8859(this->data + 2, Bytes - 2, s.data(), s.size(), true); - this->clear_after(ret.bytes_written + 2); + this->clear_after_bytes(ret.bytes_written + 2); } } else { try { auto ret = tt_utf8_to_8859(this->data, Bytes, s.data(), s.size(), true); - this->clear_after(ret.bytes_written); + this->clear_after_bytes(ret.bytes_written); } catch (const std::runtime_error&) { this->data[0] = '\t'; this->data[1] = 'J'; auto ret = tt_utf8_to_sjis(this->data + 2, Bytes - 2, s.data(), s.size(), true); - this->clear_after(ret.bytes_written + 2); + this->clear_after_bytes(ret.bytes_written + 2); } } print_data(stderr, this->data, Bytes); @@ -400,14 +382,30 @@ struct pstring { } } catch (const std::runtime_error& e) { log_warning("Unencodable text: %s", e.what()); - if (Bytes >= 3) { - this->data[0] = '<'; - this->data[1] = '?'; - this->data[2] = '>'; - this->clear_after(3); - } else if (Bytes >= 1) { - this->data[0] = '?'; - this->clear_after(1); + if (BytesPerChar == 2) { + if (Bytes >= 6) { + this->data[0] = '<'; + this->data[1] = 0x00; + this->data[2] = '?'; + this->data[3] = 0x00; + this->data[4] = '>'; + this->data[5] = 0x00; + this->clear_after_bytes(6); + } else if (Bytes >= 2) { + this->data[0] = '?'; + this->data[1] = 0x00; + this->clear_after_bytes(2); + } + } else { + if (Bytes >= 3) { + this->data[0] = '<'; + this->data[1] = '?'; + this->data[2] = '>'; + this->clear_after_bytes(3); + } else if (Bytes >= 1) { + this->data[0] = '?'; + this->clear_after_bytes(1); + } } } } @@ -539,11 +537,11 @@ struct pstring { } void clear(uint8_t v = 0) { - memset(this->data, v, Chars * BytesPerChar); + memset(this->data, v, Bytes); } - void clear_after(size_t pos, uint8_t v = 0) { - for (pos *= BytesPerChar; pos < Chars * BytesPerChar; pos++) { + void clear_after_bytes(size_t pos, uint8_t v = 0) { + for (; pos < Bytes; pos++) { this->data[pos] = v; } } @@ -557,7 +555,7 @@ struct pstring { void assign_raw(const void* data, size_t size) { memcpy(this->data, data, std::min(size, Bytes)); - this->clear_after(size); + this->clear_after_bytes(size); } void assign_raw(const std::string& data) { this->assign_raw(data.data(), data.size());