From 86ca408d469811d13a15d5c7a671feda38126eb0 Mon Sep 17 00:00:00 2001 From: rubidium Date: Fri, 6 Mar 2009 01:23:25 +0000 Subject: (svn r15626) -Fix [FS#2698]: UTF8 string handling could cause buffer overruns. --- src/console.cpp | 2 +- src/fios.cpp | 5 +++-- src/network/core/packet.cpp | 3 ++- src/saveload/oldloader.cpp | 1 + src/saveload/saveload.cpp | 2 ++ src/string.cpp | 22 ++++++++++++++++++---- src/string_func.h | 12 +++++++++--- src/video/dedicated_v.cpp | 2 +- 8 files changed, 37 insertions(+), 12 deletions(-) diff --git a/src/console.cpp b/src/console.cpp index 5cd5504b9..a113abf86 100644 --- a/src/console.cpp +++ b/src/console.cpp @@ -97,7 +97,7 @@ void IConsolePrint(ConsoleColour colour_code, const char *string) * characters and (when applicable) assign it to the console buffer */ str = strdup(string); str_strip_colours(str); - str_validate(str); + str_validate(str, str + strlen(str)); if (_network_dedicated) { fprintf(stdout, "%s\n", str); diff --git a/src/fios.cpp b/src/fios.cpp index 11ecd7941..3fb5dabe4 100644 --- a/src/fios.cpp +++ b/src/fios.cpp @@ -247,7 +247,7 @@ bool FiosFileScanner::AddFile(const char *filename, size_t basepath_length) t = (t == NULL) ? filename : (t + 1); } strecpy(fios->title, t, lastof(fios->title)); - str_validate(fios->title); + str_validate(fios->title, lastof(fios->title)); return true; } @@ -292,7 +292,7 @@ static void FiosGetFileList(SaveLoadDialogMode mode, fios_getlist_callback_proc fios->mtime = 0; strecpy(fios->name, d_name, lastof(fios->name)); snprintf(fios->title, lengthof(fios->title), "%s" PATHSEP " (Directory)", d_name); - str_validate(fios->title); + str_validate(fios->title, lastof(fios->title)); } } closedir(dir); @@ -344,6 +344,7 @@ static void GetFileTitle(const char *file, char *title, const char *last) size_t read = fread(title, 1, last - title, f); assert(title + read <= last); title[read] = '\0'; + str_validate(title, last); FioFCloseFile(f); } diff --git a/src/network/core/packet.cpp b/src/network/core/packet.cpp index 5c06ab488..013235d11 100644 --- a/src/network/core/packet.cpp +++ b/src/network/core/packet.cpp @@ -237,6 +237,7 @@ void Packet::Recv_string(char *buffer, size_t size, bool allow_newlines) { PacketSize pos; char *bufp = buffer; + const char *last = buffer + size - 1; /* Don't allow reading from a closed socket */ if (cs->HasClientQuit()) return; @@ -253,7 +254,7 @@ void Packet::Recv_string(char *buffer, size_t size, bool allow_newlines) } this->pos = pos; - str_validate(bufp, allow_newlines); + str_validate(bufp, last, allow_newlines); } #endif /* ENABLE_NETWORK */ diff --git a/src/saveload/oldloader.cpp b/src/saveload/oldloader.cpp index 7def93796..6912ca9b5 100644 --- a/src/saveload/oldloader.cpp +++ b/src/saveload/oldloader.cpp @@ -232,6 +232,7 @@ static inline bool CheckOldSavegameType(FILE *f, char *temp, const char *last, u bool ret = VerifyOldNameChecksum(temp, len); temp[len - 2] = '\0'; // name is nul-terminated in savegame, but it's better to be sure + str_validate(temp, last); return ret; } diff --git a/src/saveload/saveload.cpp b/src/saveload/saveload.cpp index 31f3b3c2d..390f36b3c 100644 --- a/src/saveload/saveload.cpp +++ b/src/saveload/saveload.cpp @@ -33,6 +33,7 @@ #include "../statusbar_gui.h" #include "../fileio_func.h" #include "../gamelog.h" +#include "../string_func.h" #include "table/strings.h" @@ -631,6 +632,7 @@ static void SlString(void *ptr, size_t length, VarType conv) } ((char*)ptr)[len] = '\0'; // properly terminate the string + str_validate((char*)ptr, (char*)ptr + len); } } diff --git a/src/string.cpp b/src/string.cpp index 52dfc260c..fc818255f 100644 --- a/src/string.cpp +++ b/src/string.cpp @@ -97,13 +97,27 @@ char *CDECL str_fmt(const char *str, ...) } -void str_validate(char *str, bool allow_newlines, bool ignore) +void str_validate(char *str, const char *last, bool allow_newlines, bool ignore) { + /* Assume the ABSOLUTE WORST to be in str as it comes from the outside. */ + char *dst = str; - WChar c; - size_t len; + while (*str != '\0') { + size_t len = Utf8EncodedCharLen(*str); + /* If the character is unknown, i.e. encoded length is 0 + * we assume worst case for the length check. + * The length check is needed to prevent Utf8Decode to read + * over the terminating '\0' if that happens to be placed + * within the encoding of an UTF8 character. */ + if ((len == 0 && str + 4 > last) || str + len > last) break; + + WChar c; + len = Utf8Decode(&c, str); + /* It's possible to encode the string termination character + * into a multiple bytes. This prevents those termination + * characters to be skipped */ + if (c == '\0') break; - for (len = Utf8Decode(&c, str); c != '\0'; len = Utf8Decode(&c, str)) { if (IsPrintable(c) && (c < SCC_SPRITE_START || c > SCC_SPRITE_END)) { /* Copy the character back. Even if dst is current the same as str * (i.e. no characters have been changed) this is quicker than diff --git a/src/string_func.h b/src/string_func.h index ad32a5558..ae5903bf1 100644 --- a/src/string_func.h +++ b/src/string_func.h @@ -93,9 +93,15 @@ int CDECL seprintf(char *str, const char *last, const char *format, ...); char *CDECL str_fmt(const char *str, ...); -/** Scans the string for valid characters and if it finds invalid ones, - * replaces them with a question mark '?' */ -void str_validate(char *str, bool allow_newlines = false, bool ignore = false); +/** + * Scans the string for valid characters and if it finds invalid ones, + * replaces them with a question mark '?' (if not ignored) + * @param str the string to validate + * @param last the last valid character of str + * @param allow_newlines whether newlines should be allowed or ignored + * @param ignore whether to ignore or replace with a question mark + */ +void str_validate(char *str, const char *last, bool allow_newlines = false, bool ignore = false); /** Scans the string for colour codes and strips them */ void str_strip_colours(char *str); diff --git a/src/video/dedicated_v.cpp b/src/video/dedicated_v.cpp index 2a997d9f9..456ee7b84 100644 --- a/src/video/dedicated_v.cpp +++ b/src/video/dedicated_v.cpp @@ -235,7 +235,7 @@ static void DedicatedHandleKeyInput() break; } } - str_validate(input_line); + str_validate(input_line, lastof(input_line)); IConsoleCmdExec(input_line); // execute command } -- cgit v1.2.3-70-g09d2