diff --git a/CHANGELOG.md b/CHANGELOG.md index be587c99a..07c02fa8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Half-Life Updated changelog +## Changes in V1.1.0 + +> Note: this update has not been released yet. + +### Bug Fixes + +* Fixed potential buffer overflows in text localization (Thanks OMAM) + ## Changes in V1.0.0 ### Bug fixes diff --git a/cl_dll/text_message.cpp b/cl_dll/text_message.cpp index 4ed642309..db266f381 100644 --- a/cl_dll/text_message.cpp +++ b/cl_dll/text_message.cpp @@ -20,6 +20,10 @@ // this class routes messages through titles.txt for localisation // +#include +#include +#include + #include "hud.h" #include "cl_util.h" #include @@ -46,22 +50,43 @@ bool CHudTextMessage::Init() // the new value is pushed into dst_buffer char* CHudTextMessage::LocaliseTextString(const char* msg, char* dst_buffer, int buffer_size) { + assert(buffer_size > 0); + char* dst = dst_buffer; - int remainingBufferSize = buffer_size; + // Subtract one so we have space for the null terminator no matter what. + std::size_t remainingBufferSize = buffer_size - 1; - for (char* src = (char*)msg; *src != 0 && remainingBufferSize > 0; remainingBufferSize--) + for (const char* src = msg; *src != '\0' && remainingBufferSize > 0; --remainingBufferSize) { if (*src == '#') { // cut msg name out of string static char word_buf[255]; - char *wdst = word_buf, *word_start = src; - for (++src; (*src >= 'A' && *src <= 'z') || (*src >= '0' && *src <= '9'); wdst++, src++) + const char* word_start = src; + + ++src; + { - *wdst = *src; + const auto end = std::find_if_not(src, src + std::strlen(src), [](auto c) + { return (c >= 'A' && c <= 'z') || (c >= '0' && c <= '9'); }); + + const std::size_t nameLength = end - src; + + const std::size_t count = std::min(std::size(word_buf) - 1, nameLength); + + if (count < nameLength) + { + gEngfuncs.Con_DPrintf( + "CHudTextMessage::LocaliseTextString: Token name starting at index %d too long in message \"%s\"\n", + static_cast(src - msg), msg); + } + + std::strncpy(word_buf, src, count); + word_buf[count] = '\0'; + + src += nameLength; } - *wdst = 0; // lookup msg name in titles.txt client_textmessage_t* clmsg = TextMessageGet(word_buf); @@ -69,26 +94,29 @@ char* CHudTextMessage::LocaliseTextString(const char* msg, char* dst_buffer, int { src = word_start; *dst = *src; - dst++, src++; + dst++; + src++; continue; } // copy string into message over the msg name - for (char* wsrc = (char*)clmsg->pMessage; *wsrc != 0; wsrc++, dst++) - { - *dst = *wsrc; - } - *dst = 0; + const std::size_t count = std::min(remainingBufferSize, std::strlen(clmsg->pMessage)); + + std::strncpy(dst, clmsg->pMessage, count); + + dst += count; + remainingBufferSize -= count; } else { *dst = *src; - dst++, src++; - *dst = 0; + dst++; + src++; } } - dst_buffer[buffer_size - 1] = 0; // ensure null termination + *dst = '\0'; // ensure null termination + return dst_buffer; } @@ -96,7 +124,7 @@ char* CHudTextMessage::LocaliseTextString(const char* msg, char* dst_buffer, int char* CHudTextMessage::BufferedLocaliseTextString(const char* msg) { static char dst_buffer[1024]; - LocaliseTextString(msg, dst_buffer, 1024); + LocaliseTextString(msg, dst_buffer, std::size(dst_buffer)); return dst_buffer; }