From 4361a6dbf4b45cd7cf88ceb9d0a1ddfc52a47f34 Mon Sep 17 00:00:00 2001 From: Jonathan G Rennison Date: Thu, 11 Mar 2021 01:08:33 +0000 Subject: [PATCH] Avoid undefined behaviour const_casting std::string c_str() Use non-const data() instead See: #224 --- src/console_cmds.cpp | 4 ++-- src/network/core/packet.cpp | 3 ++- src/network/network.cpp | 2 +- src/network/network_client.cpp | 2 +- src/network/network_server.cpp | 2 +- src/openttd.cpp | 4 ++-- src/saveload/debug_sl.cpp | 8 ++++---- src/saveload/extended_ver_sl.cpp | 2 +- src/saveload/saveload.cpp | 6 +++--- src/string_func_extra.h | 2 +- 10 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/console_cmds.cpp b/src/console_cmds.cpp index 7864023d1e..b75e8cf0f3 100644 --- a/src/console_cmds.cpp +++ b/src/console_cmds.cpp @@ -2325,7 +2325,7 @@ DEF_CONSOLE_CMD(ConDumpLoadDebugLog) } std::string dbgl = _loadgame_DBGL_data; - PrintLineByLine(const_cast(dbgl.c_str())); + PrintLineByLine(dbgl.data()); return true; } @@ -2337,7 +2337,7 @@ DEF_CONSOLE_CMD(ConDumpLoadDebugConfig) } std::string dbgc = _loadgame_DBGC_data; - PrintLineByLine(const_cast(dbgc.c_str())); + PrintLineByLine(dbgc.data()); return true; } diff --git a/src/network/core/packet.cpp b/src/network/core/packet.cpp index f78234a4ef..6be5fef11f 100644 --- a/src/network/core/packet.cpp +++ b/src/network/core/packet.cpp @@ -11,6 +11,7 @@ #include "../../stdafx.h" #include "../../string_func.h" +#include "../../string_func_extra.h" #include "../../command_type.h" #include "packet.h" @@ -337,7 +338,7 @@ void Packet::Recv_string(std::string &buffer, StringValidationSettings settings) size_t length = ttd_strnlen((const char *)(this->buffer + this->pos), this->size - this->pos - 1); buffer.assign((const char *)(this->buffer + this->pos), length); this->pos += length + 1; - str_validate(const_cast(buffer.c_str()), buffer.c_str() + buffer.size(), settings); + str_validate_inplace(buffer, settings); } /** diff --git a/src/network/network.cpp b/src/network/network.cpp index 84edab9a85..fe5d1380d9 100644 --- a/src/network/network.cpp +++ b/src/network/network.cpp @@ -946,7 +946,7 @@ void NetworkGameLoop() cp->text.resize(MAX_CMD_TEXT_LENGTH); static_assert(MAX_CMD_TEXT_LENGTH > 8192); int ret = sscanf(p, "date{%x; %x; %x}; company: %x; tile: %x (%*u x %*u); p1: %x; p2: %x; p3: " OTTD_PRINTFHEX64 "; cmd: %x; \"%8192[^\"]\"", - &next_date, &next_date_fract, &next_tick_skip_counter, &company, &cp->tile, &cp->p1, &cp->p2, &cp->p3, &cp->cmd, const_cast(cp->text.c_str())); + &next_date, &next_date_fract, &next_tick_skip_counter, &company, &cp->tile, &cp->p1, &cp->p2, &cp->p3, &cp->cmd, cp->text.data()); /* There are 10 pieces of data to read, however the last is a * string that might or might not exist. Ignore it if that * string misses because in 99% of the time it's not used. */ diff --git a/src/network/network_client.cpp b/src/network/network_client.cpp index 73198401b0..ddb5734ece 100644 --- a/src/network/network_client.cpp +++ b/src/network/network_client.cpp @@ -1163,7 +1163,7 @@ NetworkRecvStatus ClientNetworkGameSocketHandler::Receive_SERVER_DESYNC_LOG(Pack { uint size = p->Recv_uint16(); this->server_desync_log.resize(this->server_desync_log.size() + size); - p->Recv_binary(const_cast(this->server_desync_log.data() + this->server_desync_log.size() - size), size); + p->Recv_binary(this->server_desync_log.data() + this->server_desync_log.size() - size, size); DEBUG(net, 2, "Received %u bytes of server desync log", size); return NETWORK_RECV_STATUS_OKAY; } diff --git a/src/network/network_server.cpp b/src/network/network_server.cpp index 5277928a5b..220d5a78bd 100644 --- a/src/network/network_server.cpp +++ b/src/network/network_server.cpp @@ -1280,7 +1280,7 @@ NetworkRecvStatus ServerNetworkGameSocketHandler::Receive_CLIENT_DESYNC_LOG(Pack { uint size = p->Recv_uint16(); this->desync_log.resize(this->desync_log.size() + size); - p->Recv_binary(const_cast(this->desync_log.data() + this->desync_log.size() - size), size); + p->Recv_binary(this->desync_log.data() + this->desync_log.size() - size, size); DEBUG(net, 2, "Received %u bytes of client desync log", size); this->receive_limit += p->size; return NETWORK_RECV_STATUS_OKAY; diff --git a/src/openttd.cpp b/src/openttd.cpp index c5f75d7f59..aa590122a7 100644 --- a/src/openttd.cpp +++ b/src/openttd.cpp @@ -353,7 +353,7 @@ static void WriteSavegameDebugData(const char *name) if (_load_check_data.debug_log_data.size()) { p += seprintf(p, buflast, "%u bytes of debug log data in savegame\n", (uint) _load_check_data.debug_log_data.size()); std::string buffer = _load_check_data.debug_log_data; - ProcessLineByLine(const_cast(buffer.data()), [&](const char *line) { + ProcessLineByLine(buffer.data(), [&](const char *line) { if (buflast - p <= 1024) bump_size(); p += seprintf(p, buflast, "> %s\n", line); }); @@ -363,7 +363,7 @@ static void WriteSavegameDebugData(const char *name) if (_load_check_data.debug_config_data.size()) { p += seprintf(p, buflast, "%u bytes of debug config data in savegame\n", (uint) _load_check_data.debug_config_data.size()); std::string buffer = _load_check_data.debug_config_data; - ProcessLineByLine(const_cast(buffer.data()), [&](const char *line) { + ProcessLineByLine(buffer.data(), [&](const char *line) { if (buflast - p <= 1024) bump_size(); p += seprintf(p, buflast, "> %s\n", line); }); diff --git a/src/saveload/debug_sl.cpp b/src/saveload/debug_sl.cpp index fca47f5565..79bbf27e4c 100644 --- a/src/saveload/debug_sl.cpp +++ b/src/saveload/debug_sl.cpp @@ -32,7 +32,7 @@ static void Load_DBGL() size_t length = SlGetFieldLength(); if (length) { _loadgame_DBGL_data.resize(length); - ReadBuffer::GetCurrent()->CopyBytes(reinterpret_cast(const_cast(_loadgame_DBGL_data.data())), length); + ReadBuffer::GetCurrent()->CopyBytes(reinterpret_cast(_loadgame_DBGL_data.data()), length); } } @@ -45,7 +45,7 @@ static void Check_DBGL() size_t length = SlGetFieldLength(); if (length) { _load_check_data.debug_log_data.resize(length); - ReadBuffer::GetCurrent()->CopyBytes(reinterpret_cast(const_cast(_load_check_data.debug_log_data.data())), length); + ReadBuffer::GetCurrent()->CopyBytes(reinterpret_cast(_load_check_data.debug_log_data.data()), length); } } @@ -69,7 +69,7 @@ static void Load_DBGC() size_t length = SlGetFieldLength(); if (length) { _loadgame_DBGC_data.resize(length); - ReadBuffer::GetCurrent()->CopyBytes(reinterpret_cast(const_cast(_loadgame_DBGC_data.data())), length); + ReadBuffer::GetCurrent()->CopyBytes(reinterpret_cast(_loadgame_DBGC_data.data()), length); } } @@ -82,7 +82,7 @@ static void Check_DBGC() size_t length = SlGetFieldLength(); if (length) { _load_check_data.debug_config_data.resize(length); - ReadBuffer::GetCurrent()->CopyBytes(reinterpret_cast(const_cast(_load_check_data.debug_config_data.data())), length); + ReadBuffer::GetCurrent()->CopyBytes(reinterpret_cast(_load_check_data.debug_config_data.data()), length); } } diff --git a/src/saveload/extended_ver_sl.cpp b/src/saveload/extended_ver_sl.cpp index 177d35b489..38ca84a4a1 100644 --- a/src/saveload/extended_ver_sl.cpp +++ b/src/saveload/extended_ver_sl.cpp @@ -635,7 +635,7 @@ static void Load_SLXI() static void loadVL(const SlxiSubChunkInfo *info, uint32 length) { _sl_xv_version_label.resize(length); - ReadBuffer::GetCurrent()->CopyBytes(reinterpret_cast(const_cast(_sl_xv_version_label.c_str())), length); + ReadBuffer::GetCurrent()->CopyBytes(reinterpret_cast(_sl_xv_version_label.data()), length); } static uint32 saveVL(const SlxiSubChunkInfo *info, bool dry_run) diff --git a/src/saveload/saveload.cpp b/src/saveload/saveload.cpp index 3bf160af59..a03d669966 100644 --- a/src/saveload/saveload.cpp +++ b/src/saveload/saveload.cpp @@ -1132,20 +1132,20 @@ static void SlStdString(std::string &str, VarType conv) switch (_sl.action) { case SLA_SAVE: { SlWriteArrayLength(str.size()); - SlCopyBytes(const_cast(str.data()), str.size()); + SlCopyBytes(str.data(), str.size()); break; } case SLA_LOAD_CHECK: case SLA_LOAD: { size_t len = SlReadArrayLength(); str.resize(len); - SlCopyBytes(const_cast(str.c_str()), len); + SlCopyBytes(str.data(), len); StringValidationSettings settings = SVS_REPLACE_WITH_QUESTION_MARK; if ((conv & SLF_ALLOW_CONTROL) != 0) { settings = settings | SVS_ALLOW_CONTROL_CODE; if (IsSavegameVersionBefore(SLV_169)) { - char *buf = const_cast(str.c_str()); + char *buf = str.data(); str.resize(str_fix_scc_encoded(buf, buf + str.size()) - buf); } } diff --git a/src/string_func_extra.h b/src/string_func_extra.h index 71aeb5680b..5ac5d7ed67 100644 --- a/src/string_func_extra.h +++ b/src/string_func_extra.h @@ -14,7 +14,7 @@ static inline void str_validate_inplace(std::string &str, StringValidationSettings settings = SVS_REPLACE_WITH_QUESTION_MARK) { if (str.empty()) return; - char *buf = const_cast(str.c_str()); + char *buf = str.data(); str.resize(str_validate(buf, buf + str.size(), settings) - buf); }