summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorrubidium <rubidium@openttd.org>2009-03-06 01:23:25 +0000
committerrubidium <rubidium@openttd.org>2009-03-06 01:23:25 +0000
commit86ca408d469811d13a15d5c7a671feda38126eb0 (patch)
tree8972c2c7b9b1ab7c2b41b46cd9a44f1bb0fbd9bd
parentc3a7e6b693716232fd2aefab3f36bd555620b563 (diff)
downloadopenttd-86ca408d469811d13a15d5c7a671feda38126eb0.tar.xz
(svn r15626) -Fix [FS#2698]: UTF8 string handling could cause buffer overruns.
-rw-r--r--src/console.cpp2
-rw-r--r--src/fios.cpp5
-rw-r--r--src/network/core/packet.cpp3
-rw-r--r--src/saveload/oldloader.cpp1
-rw-r--r--src/saveload/saveload.cpp2
-rw-r--r--src/string.cpp22
-rw-r--r--src/string_func.h12
-rw-r--r--src/video/dedicated_v.cpp2
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
}