From c10f7db5760594b1166872746eb0769232ed7b77 Mon Sep 17 00:00:00 2001 From: smatz Date: Wed, 22 Oct 2008 19:12:10 +0000 Subject: (svn r14514) -Codechange: use 'size' instead of 'length' for querystring and textbuf, explicitly say it includes the terminating zero -Fix: one couldn't rename things with too long default/automatic name -Fix: buffer overflow in console when too long (1024 bytes) command was entered --- src/console_gui.cpp | 6 ++-- src/genworld_gui.cpp | 1 + src/misc_gui.cpp | 68 +++++++++++++++++++++------------------- src/network/network_chat_gui.cpp | 4 +-- src/querystring_gui.h | 3 +- src/signs_gui.cpp | 2 +- src/textbuf_gui.h | 14 ++++----- 7 files changed, 52 insertions(+), 46 deletions(-) diff --git a/src/console_gui.cpp b/src/console_gui.cpp index 8d4dfc2cf..baa537322 100644 --- a/src/console_gui.cpp +++ b/src/console_gui.cpp @@ -133,7 +133,7 @@ IConsoleModes _iconsole_mode; static void IConsoleClearCommand() { memset(_iconsole_cmdline.buf, 0, ICON_CMDLN_SIZE); - _iconsole_cmdline.length = 0; + _iconsole_cmdline.size = 1; // only terminating zero _iconsole_cmdline.width = 0; _iconsole_cmdline.caretpos = 0; _iconsole_cmdline.caretxoffs = 0; @@ -336,7 +336,7 @@ void IConsoleGUIInit() memset(_iconsole_history, 0, sizeof(_iconsole_history)); _iconsole_cmdline.buf = CallocT(ICON_CMDLN_SIZE); // create buffer and zero it - _iconsole_cmdline.maxlength = ICON_CMDLN_SIZE; + _iconsole_cmdline.maxsize = ICON_CMDLN_SIZE; IConsolePrintF(CC_WARNING, "OpenTTD Game Console Revision 7 - %s", _openttd_revision); IConsolePrint(CC_WHITE, "------------------------------------"); @@ -430,7 +430,7 @@ static void IConsoleHistoryNavigate(int direction) IConsoleClearCommand(); /* copy history to 'command prompt / bash' */ assert(_iconsole_history[i] != NULL && IsInsideMM(i, 0, ICON_HISTORY_SIZE)); - ttd_strlcpy(_iconsole_cmdline.buf, _iconsole_history[i], _iconsole_cmdline.maxlength); + ttd_strlcpy(_iconsole_cmdline.buf, _iconsole_history[i], _iconsole_cmdline.maxsize); UpdateTextBufferSize(&_iconsole_cmdline); } diff --git a/src/genworld_gui.cpp b/src/genworld_gui.cpp index 565a898c3..6091d75b6 100644 --- a/src/genworld_gui.cpp +++ b/src/genworld_gui.cpp @@ -255,6 +255,7 @@ struct GenerateLandscapeWindow : public QueryStringBaseWindow { { this->LowerWidget(_settings_newgame.game_creation.landscape + GLAND_TEMPERATE); + /* snprintf() always outputs trailing '\0', so whole buffer can be used */ snprintf(this->edit_str_buf, this->edit_str_size, "%u", _settings_newgame.game_creation.generation_seed); InitializeTextBuffer(&this->text, this->edit_str_buf, this->edit_str_size, 120); this->caption = STR_NULL; diff --git a/src/misc_gui.cpp b/src/misc_gui.cpp index e9d374990..76061461f 100644 --- a/src/misc_gui.cpp +++ b/src/misc_gui.cpp @@ -793,8 +793,8 @@ static void DelChar(Textbuf *tb, bool backspace) } /* Move the remaining characters over the marker */ - memmove(s, s + len, tb->length - (s - tb->buf) - len + 1); - tb->length -= len; + memmove(s, s + len, tb->size - (s - tb->buf) - len); + tb->size -= len; } /** @@ -809,7 +809,7 @@ bool DeleteTextBufferChar(Textbuf *tb, int delmode) if (delmode == WKC_BACKSPACE && tb->caretpos != 0) { DelChar(tb, true); return true; - } else if (delmode == WKC_DELETE && tb->caretpos < tb->length) { + } else if (delmode == WKC_DELETE && tb->caretpos < tb->size - 1) { DelChar(tb, false); return true; } @@ -823,9 +823,9 @@ bool DeleteTextBufferChar(Textbuf *tb, int delmode) */ void DeleteTextBufferAll(Textbuf *tb) { - memset(tb->buf, 0, tb->maxlength); - tb->length = tb->width = 0; - tb->caretpos = tb->caretxoffs = 0; + memset(tb->buf, 0, tb->maxsize); + tb->size = 1; + tb->width = tb->caretpos = tb->caretxoffs = 0; } /** @@ -840,11 +840,11 @@ bool InsertTextBufferChar(Textbuf *tb, WChar key) { const byte charwidth = GetCharacterWidth(FS_NORMAL, key); uint16 len = (uint16)Utf8CharLen(key); - if (tb->length < (tb->maxlength - len) && (tb->maxwidth == 0 || tb->width + charwidth <= tb->maxwidth)) { - memmove(tb->buf + tb->caretpos + len, tb->buf + tb->caretpos, tb->length - tb->caretpos + 1); + if (tb->size + len <= tb->maxsize && (tb->maxwidth == 0 || tb->width + charwidth <= tb->maxwidth)) { + memmove(tb->buf + tb->caretpos + len, tb->buf + tb->caretpos, tb->size - tb->caretpos); Utf8Encode(tb->buf + tb->caretpos, key); - tb->length += len; - tb->width += charwidth; + tb->size += len; + tb->width += charwidth; tb->caretpos += len; tb->caretxoffs += charwidth; @@ -876,7 +876,7 @@ bool MoveTextBufferPos(Textbuf *tb, int navmode) break; case WKC_RIGHT: - if (tb->caretpos < tb->length) { + if (tb->caretpos < tb->size - 1) { WChar c; tb->caretpos += (uint16)Utf8Decode(&c, tb->buf + tb->caretpos); @@ -892,7 +892,7 @@ bool MoveTextBufferPos(Textbuf *tb, int navmode) return true; case WKC_END: - tb->caretpos = tb->length; + tb->caretpos = tb->size - 1; tb->caretxoffs = tb->width; return true; @@ -908,16 +908,18 @@ bool MoveTextBufferPos(Textbuf *tb, int navmode) * and the maximum length of this buffer * @param tb Textbuf type which is getting initialized * @param buf the buffer that will be holding the data for input - * @param maxlength maximum length in characters of this buffer + * @param maxsize maximum size in bytes, including terminating '\0' * @param maxwidth maximum length in pixels of this buffer. If reached, buffer - * cannot grow, even if maxlength would allow because there is space. A length - * of zero '0' means the buffer is only restricted by maxlength */ -void InitializeTextBuffer(Textbuf *tb, const char *buf, uint16 maxlength, uint16 maxwidth) + * cannot grow, even if maxsize would allow because there is space. Width + * of zero '0' means the buffer is only restricted by maxsize */ +void InitializeTextBuffer(Textbuf *tb, char *buf, uint16 maxsize, uint16 maxwidth) { - tb->buf = (char*)buf; - tb->maxlength = maxlength; - tb->maxwidth = maxwidth; - tb->caret = true; + assert(maxsize != 0); + + tb->buf = buf; + tb->maxsize = maxsize; + tb->maxwidth = maxwidth; + tb->caret = true; UpdateTextBufferSize(tb); } @@ -930,17 +932,19 @@ void InitializeTextBuffer(Textbuf *tb, const char *buf, uint16 maxlength, uint16 void UpdateTextBufferSize(Textbuf *tb) { const char *buf = tb->buf; - WChar c = Utf8Consume(&buf); tb->width = 0; - tb->length = 0; + tb->size = 1; // terminating zero - for (; c != '\0' && tb->length < (tb->maxlength - 1); c = Utf8Consume(&buf)) { + WChar c; + while ((c = Utf8Consume(&buf)) != '\0') { tb->width += GetCharacterWidth(FS_NORMAL, c); - tb->length += Utf8CharLen(c); + tb->size += Utf8CharLen(c); } - tb->caretpos = tb->length; + assert(tb->size <= tb->maxsize); + + tb->caretpos = tb->size - 1; tb->caretxoffs = tb->width; } @@ -1163,22 +1167,22 @@ static const WindowDesc _query_string_desc = { /** Show a query popup window with a textbox in it. * @param str StringID for the text shown in the textbox * @param caption StringID of text shown in caption of querywindow - * @param maxlen maximum length in characters allowed + * @param maxsize maximum size in bytes (including terminating '\0') * @param maxwidth maximum width in pixels allowed * @param parent pointer to a Window that will handle the events (ok/cancel) of this * window. If NULL, results are handled by global function HandleOnEditText * @param afilter filters out unwanted character input * @param flags various flags, @see QueryStringFlags */ -void ShowQueryString(StringID str, StringID caption, uint maxlen, uint maxwidth, Window *parent, CharSetFilter afilter, QueryStringFlags flags) +void ShowQueryString(StringID str, StringID caption, uint maxsize, uint maxwidth, Window *parent, CharSetFilter afilter, QueryStringFlags flags) { DeleteWindowById(WC_QUERY_STRING, 0); DeleteWindowById(WC_SAVELOAD, 0); - QueryStringWindow *w = new QueryStringWindow(maxlen + 1, &_query_string_desc, parent); + QueryStringWindow *w = new QueryStringWindow(maxsize, &_query_string_desc, parent); - GetString(w->edit_str_buf, str, &w->edit_str_buf[maxlen]); - w->edit_str_buf[maxlen] = '\0'; + GetString(w->edit_str_buf, str, &w->edit_str_buf[maxsize - 1]); + w->edit_str_buf[maxsize - 1] = '\0'; if ((flags & QSF_ACCEPT_UNCHANGED) == 0) w->orig = strdup(w->edit_str_buf); @@ -1194,7 +1198,7 @@ void ShowQueryString(StringID str, StringID caption, uint maxlen, uint maxwidth, w->LowerWidget(QUERY_STR_WIDGET_TEXT); w->caption = caption; w->afilter = afilter; - InitializeTextBuffer(&w->text, w->edit_str_buf, maxlen, maxwidth); + InitializeTextBuffer(&w->text, w->edit_str_buf, maxsize, maxwidth); } @@ -1583,7 +1587,7 @@ struct SaveLoadWindow : public QueryStringBaseWindow { ShowHeightmapLoad(); } else { /* SLD_SAVE_GAME, SLD_SAVE_SCENARIO copy clicked name to editbox */ - ttd_strlcpy(this->text.buf, file->title, this->text.maxlength); + ttd_strlcpy(this->text.buf, file->title, this->text.maxsize); UpdateTextBufferSize(&this->text); this->InvalidateWidget(10); } diff --git a/src/network/network_chat_gui.cpp b/src/network/network_chat_gui.cpp index 552095ec9..12cda3a43 100644 --- a/src/network/network_chat_gui.cpp +++ b/src/network/network_chat_gui.cpp @@ -374,11 +374,11 @@ struct NetworkChatWindow : public QueryStringBaseWindow { /* If we are completing at the begin of the line, skip the ': ' we added */ if (tb_buf == pre_buf) { offset = 0; - length = tb->length - 2; + length = (tb->size - 1) - 2; } else { /* Else, find the place we are completing at */ offset = strlen(pre_buf) + 1; - length = tb->length - offset; + length = (tb->size - 1) - offset; } /* Compare if we have a match */ diff --git a/src/querystring_gui.h b/src/querystring_gui.h index 05a25c42a..9c033200f 100644 --- a/src/querystring_gui.h +++ b/src/querystring_gui.h @@ -41,10 +41,11 @@ struct QueryString { struct QueryStringBaseWindow : public Window, public QueryString { char *edit_str_buf; char *orig_str_buf; - const uint16 edit_str_size; + const uint16 edit_str_size; ///< maximum length of string (in bytes), including terminating '\0' QueryStringBaseWindow(uint16 size, const WindowDesc *desc, WindowNumber window_number = 0) : Window(desc, window_number), edit_str_size(size) { + assert(size != 0); this->edit_str_buf = CallocT(size); } diff --git a/src/signs_gui.cpp b/src/signs_gui.cpp index 714299f62..925092e00 100644 --- a/src/signs_gui.cpp +++ b/src/signs_gui.cpp @@ -215,7 +215,7 @@ struct SignWindow : QueryStringBaseWindow, SignList { void UpdateSignEditWindow(const Sign *si) { - char *last_of = &this->edit_str_buf[this->edit_str_size - 1]; + char *last_of = &this->edit_str_buf[this->edit_str_size - 1]; // points to terminating '\0' /* Display an empty string when the sign hasnt been edited yet */ if (si->name != NULL) { diff --git a/src/textbuf_gui.h b/src/textbuf_gui.h index 7d91b67e7..26ae37a59 100644 --- a/src/textbuf_gui.h +++ b/src/textbuf_gui.h @@ -11,12 +11,12 @@ #include "core/enum_type.hpp" struct Textbuf { - char *buf; ///< buffer in which text is saved - uint16 maxlength, maxwidth; ///< the maximum size of the buffer. Maxwidth specifies screensize in pixels, maxlength is in bytes - uint16 length, width; ///< the current size of the string. Width specifies screensize in pixels, length is in bytes - bool caret; ///< is the caret ("_") visible or not - uint16 caretpos; ///< the current position of the caret in the buffer, in bytes - uint16 caretxoffs; ///< the current position of the caret in pixels + char *buf; ///< buffer in which text is saved + uint16 maxsize, maxwidth; ///< the maximum size of the buffer. Maxwidth specifies screensize in pixels, maxsize is in bytes (including terminating '\0') + uint16 size, width; ///< the current size of the string. Width specifies screensize in pixels, size is in bytes + bool caret; ///< is the caret ("_") visible or not + uint16 caretpos; ///< the current position of the caret in the buffer, in bytes + uint16 caretxoffs; ///< the current position of the caret in pixels }; bool HandleCaret(Textbuf *tb); @@ -26,7 +26,7 @@ bool DeleteTextBufferChar(Textbuf *tb, int delmode); bool InsertTextBufferChar(Textbuf *tb, uint32 key); bool InsertTextBufferClipboard(Textbuf *tb); bool MoveTextBufferPos(Textbuf *tb, int navmode); -void InitializeTextBuffer(Textbuf *tb, const char *buf, uint16 maxlength, uint16 maxwidth); +void InitializeTextBuffer(Textbuf *tb, char *buf, uint16 maxsize, uint16 maxwidth); void UpdateTextBufferSize(Textbuf *tb); /** Flags used in ShowQueryString() call */ -- cgit v1.2.3-70-g09d2