diff options
author | Simon Howard | 2014-04-01 21:49:16 -0400 |
---|---|---|
committer | Simon Howard | 2014-04-01 21:49:16 -0400 |
commit | a9d9335b20a0b708fae1b978f70348aec998356a (patch) | |
tree | 9d775843fddbffaa9babdf54221b0ac98105dc91 | |
parent | 17c14e1fad6dc277a6e58e4f421d5c65e210d1fe (diff) | |
download | chocolate-doom-a9d9335b20a0b708fae1b978f70348aec998356a.tar.gz chocolate-doom-a9d9335b20a0b708fae1b978f70348aec998356a.tar.bz2 chocolate-doom-a9d9335b20a0b708fae1b978f70348aec998356a.zip |
textscreen: Use safe string functions.
Define TXT_{StringCopy,StringConcat,snprintf,vsnprintf} as analogs of
the m_misc.c versions so that the textscreen library does not need a
dependency on the Doom code, and change all textscreen code to use
these instead of unsafe functions. This fixes #372.
-rw-r--r-- | src/setup/txt_keyinput.c | 2 | ||||
-rw-r--r-- | textscreen/examples/calculator.c | 6 | ||||
-rw-r--r-- | textscreen/examples/guitest.c | 14 | ||||
-rw-r--r-- | textscreen/txt_desktop.c | 2 | ||||
-rw-r--r-- | textscreen/txt_fileselect.c | 49 | ||||
-rw-r--r-- | textscreen/txt_inputbox.c | 12 | ||||
-rw-r--r-- | textscreen/txt_inputbox.h | 1 | ||||
-rw-r--r-- | textscreen/txt_main.h | 26 | ||||
-rw-r--r-- | textscreen/txt_sdl.c | 67 | ||||
-rw-r--r-- | textscreen/txt_spinctrl.c | 23 | ||||
-rw-r--r-- | textscreen/txt_spinctrl.h | 1 | ||||
-rw-r--r-- | textscreen/txt_utf8.c | 1 | ||||
-rw-r--r-- | textscreen/txt_utf8.h | 2 | ||||
-rw-r--r-- | textscreen/txt_window.c | 2 | ||||
-rw-r--r-- | textscreen/txt_window_action.c | 4 |
15 files changed, 144 insertions, 68 deletions
diff --git a/src/setup/txt_keyinput.c b/src/setup/txt_keyinput.c index f5f7b562..a35f22e1 100644 --- a/src/setup/txt_keyinput.c +++ b/src/setup/txt_keyinput.c @@ -115,7 +115,7 @@ static void TXT_KeyInputDrawer(TXT_UNCAST_ARG(key_input)) } else { - TXT_GetKeyDescription(*key_input->variable, buf); + TXT_GetKeyDescription(*key_input->variable, buf, sizeof(buf)); } TXT_SetWidgetBG(key_input); diff --git a/textscreen/examples/calculator.c b/textscreen/examples/calculator.c index 7c77e838..3db60ef7 100644 --- a/textscreen/examples/calculator.c +++ b/textscreen/examples/calculator.c @@ -49,7 +49,7 @@ void UpdateInputBox(void) { char buf[20]; - sprintf(buf, " %i", input_value); + TXT_snprintf(buf, sizeof(buf), " %i", input_value); TXT_SetLabel(input_box, buf); } @@ -76,7 +76,7 @@ void AddNumberButton(txt_table_t *table, int value) val_copy = malloc(sizeof(int)); *val_copy = value; - sprintf(buf, " %i ", value); + TXT_snprintf(buf, sizeof(buf), " %i ", value); TXT_AddWidget(table, TXT_NewButton2(buf, InsertNumber, val_copy)); } @@ -98,7 +98,7 @@ void AddOperatorButton(txt_table_t *table, char *label, operator_t op) op_copy = malloc(sizeof(operator_t)); *op_copy = op; - sprintf(buf, " %s ", label); + TXT_snprintf(buf, sizeof(buf), " %s ", label); TXT_AddWidget(table, TXT_NewButton2(buf, Operator, op_copy)); } diff --git a/textscreen/examples/guitest.c b/textscreen/examples/guitest.c index 08faae98..3904ffa9 100644 --- a/textscreen/examples/guitest.c +++ b/textscreen/examples/guitest.c @@ -76,13 +76,13 @@ void UpdateLabel(TXT_UNCAST_ARG(widget), void *user_data) { char buf[40]; - strcpy(buf, " Current value: "); + TXT_StringCopy(buf, " Current value: ", sizeof(buf)); if (cheesy) { - strcat(buf, "Cheesy "); + TXT_StringConcat(buf, "Cheesy ", sizeof(buf)); } - strcat(buf, radio_values[radiobutton_value]); - strcat(buf, "\n"); + TXT_StringConcat(buf, radio_values[radiobutton_value], sizeof(buf)); + TXT_StringConcat(buf, "\n", sizeof(buf)); TXT_SetLabel(value_label, buf); } @@ -119,11 +119,11 @@ void SetupWindow(void) for (i=0; i<5; ++i) { - sprintf(buf, "Option %i in a table:", i + 1); + TXT_snprintf(buf, sizeof(buf), "Option %i in a table:", i + 1); TXT_AddWidget(table, TXT_NewLabel(buf)); - sprintf(buf, " Button %i-1 ", i + 1); + TXT_snprintf(buf, sizeof(buf), " Button %i-1 ", i + 1); TXT_AddWidget(table, TXT_NewButton(buf)); - sprintf(buf, " Button %i-2 ", i + 1); + TXT_snprintf(buf, sizeof(buf), " Button %i-2 ", i + 1); TXT_AddWidget(table, TXT_NewButton(buf)); } diff --git a/textscreen/txt_desktop.c b/textscreen/txt_desktop.c index 2316145f..31577ec8 100644 --- a/textscreen/txt_desktop.c +++ b/textscreen/txt_desktop.c @@ -263,7 +263,7 @@ void TXT_DrawASCIITable(void) n = y * 16 + x; TXT_GotoXY(x * 5, y); - sprintf(buf, "%02x ", n); + TXT_snprintf(buf, sizeof(buf), "%02x ", n); TXT_Puts(buf); // Write the character directly to the screen memory buffer: diff --git a/textscreen/txt_fileselect.c b/textscreen/txt_fileselect.c index 0ef404b4..cec21937 100644 --- a/textscreen/txt_fileselect.c +++ b/textscreen/txt_fileselect.c @@ -217,6 +217,7 @@ static char *GenerateFilterString(char **extensions) unsigned int result_len = 1; unsigned int i; char *result, *out; + size_t out_len, offset; if (extensions == NULL) { @@ -229,15 +230,18 @@ static char *GenerateFilterString(char **extensions) } result = malloc(result_len); - out = result; + out = result; out_len = result_len; for (i = 0; extensions[i] != NULL; ++i) { // .wad files (*.wad)\0 - out += 1 + sprintf(out, "%s files (*.%s)", - extensions[i], extensions[i]); + offset = TXT_snprintf(out, out_len, "%s files (*.%s)", + extensions[i], extensions[i]); + out += offset + 1; out_len -= offset + 1; + // *.wad\0 - out += 1 + sprintf(out, "*.%s", extensions[i]); + offset = TXT_snprintf(out, out_len, "*.%s", extensions[i]); + out_len += offset + 1; out_len -= offset + 1; } *out = '\0'; @@ -378,19 +382,19 @@ static char *ExtensionsList(char **extensions) } result = malloc(result_len); - strcpy(result, "{"); + TXT_StringCopy(result, "{", result_len); for (i = 0; extensions[i] != NULL; ++i) { escaped = EscapedString(extensions[i]); - strcat(result, escaped); + TXT_StringConcat(result, escaped, result_len); free(escaped); if (extensions[i + 1] != NULL) - strcat(result, ","); + TXT_StringConcat(result, ",", result_len); } - strcat(result, "}"); + TXT_StringConcat(result, "}", result_len); return result; } @@ -427,19 +431,19 @@ static char *GenerateSelector(char *window_title, char **extensions) result = malloc(result_len); - strcpy(result, chooser); + TXT_StringCopy(result, chooser, result_len); if (window_title != NULL) { - strcat(result, " with prompt "); - strcat(result, window_title); + TXT_StringConcat(result, " with prompt ", result_len); + TXT_StringConcat(result, window_title, result_len); free(window_title); } if (ext_list != NULL) { - strcat(result, "of type "); - strcat(result, ext_list); + TXT_StringConcat(result, "of type ", result_len); + TXT_StringConcat(result, ext_list, result_len); free(ext_list); } @@ -449,11 +453,13 @@ static char *GenerateSelector(char *window_title, char **extensions) static char *GenerateAppleScript(char *window_title, char **extensions) { char *selector, *result; + size_t result_len; selector = GenerateSelector(window_title, extensions); - result = malloc(strlen(APPLESCRIPT_WRAPPER) + strlen(selector)); - sprintf(result, APPLESCRIPT_WRAPPER, selector); + result_len = strlen(APPLESCRIPT_WRAPPER) + strlen(selector); + result = malloc(result_len); + TXT_snprintf(result, result_len, APPLESCRIPT_WRAPPER, selector); free(selector); return result; @@ -515,6 +521,7 @@ int TXT_CanSelectFiles(void) char *TXT_SelectFile(char *window_title, char **extensions) { unsigned int i; + size_t len; char *result; char **argv; int argc; @@ -531,8 +538,9 @@ char *TXT_SelectFile(char *window_title, char **extensions) if (window_title != NULL) { - argv[argc] = malloc(10 + strlen(window_title)); - sprintf(argv[argc], "--title=%s", window_title); + len = 10 + strlen(window_title); + argv[argc] = malloc(len); + TXT_snprintf(argv[argc], len, "--title=%s", window_title); ++argc; } @@ -545,9 +553,10 @@ char *TXT_SelectFile(char *window_title, char **extensions) { for (i = 0; extensions[i] != NULL; ++i) { - argv[argc] = malloc(30 + strlen(extensions[i]) * 2); - sprintf(argv[argc], "--file-filter=.%s | *.%s", - extensions[i], extensions[i]); + len = 30 + strlen(extensions[i]) * 2; + argv[argc] = malloc(len); + TXT_snprintf(argv[argc], len, "--file-filter=.%s | *.%s", + extensions[i], extensions[i]); ++argc; } } diff --git a/textscreen/txt_inputbox.c b/textscreen/txt_inputbox.c index 13d35aef..df3bbbf7 100644 --- a/textscreen/txt_inputbox.c +++ b/textscreen/txt_inputbox.c @@ -44,18 +44,17 @@ static void SetBufferFromValue(txt_inputbox_t *inputbox) if (*value != NULL) { - strncpy(inputbox->buffer, *value, inputbox->size); - inputbox->buffer[inputbox->size] = '\0'; + TXT_StringCopy(inputbox->buffer, *value, inputbox->size); } else { - strcpy(inputbox->buffer, ""); + TXT_StringCopy(inputbox->buffer, "", inputbox->buffer_len); } } else if (inputbox->widget.widget_class == &txt_int_inputbox_class) { int *value = (int *) inputbox->value; - sprintf(inputbox->buffer, "%i", *value); + TXT_snprintf(inputbox->buffer, inputbox->buffer_len, "%i", *value); } } @@ -65,7 +64,7 @@ static void StartEditing(txt_inputbox_t *inputbox) if (inputbox->widget.widget_class == &txt_int_inputbox_class) { - strcpy(inputbox->buffer, ""); + TXT_StringCopy(inputbox->buffer, "", inputbox->buffer_len); } else { @@ -322,7 +321,8 @@ static txt_inputbox_t *NewInputBox(txt_widget_class_t *widget_class, // 'size' is the maximum number of characters that can be entered, // but for a UTF-8 string, each character can take up to four // characters. - inputbox->buffer = malloc(size * 4 + 1); + inputbox->buffer_len = size * 4 + 1; + inputbox->buffer = malloc(inputbox->buffer_len); inputbox->editing = 0; return inputbox; diff --git a/textscreen/txt_inputbox.h b/textscreen/txt_inputbox.h index 6237bc91..1e270d97 100644 --- a/textscreen/txt_inputbox.h +++ b/textscreen/txt_inputbox.h @@ -45,6 +45,7 @@ struct txt_inputbox_s { txt_widget_t widget; char *buffer; + size_t buffer_len; unsigned int size; int editing; void *value; diff --git a/textscreen/txt_main.h b/textscreen/txt_main.h index a6dcc954..d882b521 100644 --- a/textscreen/txt_main.h +++ b/textscreen/txt_main.h @@ -109,55 +109,55 @@ typedef enum // Initialize the screen // Returns 1 if successful, 0 if failed. - int TXT_Init(void); // Shut down text mode emulation - void TXT_Shutdown(void); // Get a pointer to the buffer containing the raw screen data. - unsigned char *TXT_GetScreenData(void); // Update an area of the screen - void TXT_UpdateScreenArea(int x, int y, int w, int h); // Update the whole screen - void TXT_UpdateScreen(void); // Read a character from the keyboard - int TXT_GetChar(void); // Read the current state of modifier keys that are held down. - int TXT_GetModifierState(txt_modifier_t mod); // Provides a short description of a key code, placing into the // provided buffer. - -void TXT_GetKeyDescription(int key, char *buf); +void TXT_GetKeyDescription(int key, char *buf, size_t buf_len); // Retrieve the current position of the mouse - void TXT_GetMousePosition(int *x, int *y); // Sleep until an event is received or the screen needs updating // Optional timeout in ms (timeout == 0 : sleep forever) - void TXT_Sleep(int timeout); // Controls whether keys are returned from TXT_GetChar based on keyboard // mapping, or raw key code. - void TXT_EnableKeyMapping(int enable); // Set the window title of the window containing the text mode screen - void TXT_SetWindowTitle(char *title); +// Safe string copy. +void TXT_StringCopy(char *dest, const char *src, size_t dest_len); + +// Safe string concatenate. +void TXT_StringConcat(char *dest, const char *src, size_t dest_len); + +// Safe version of vsnprintf(). +int TXT_vsnprintf(char *buf, size_t buf_len, const char *s, va_list args); + +// Safe version of snprintf(). +int TXT_snprintf(char *buf, size_t buf_len, const char *s, ...); + #endif /* #ifndef TXT_MAIN_H */ diff --git a/textscreen/txt_sdl.c b/textscreen/txt_sdl.c index 0fb8d722..860adfa6 100644 --- a/textscreen/txt_sdl.c +++ b/textscreen/txt_sdl.c @@ -751,7 +751,7 @@ static const char *SpecialKeyName(int key) } } -void TXT_GetKeyDescription(int key, char *buf) +void TXT_GetKeyDescription(int key, char *buf, size_t buf_len) { const char *keyname; @@ -759,15 +759,15 @@ void TXT_GetKeyDescription(int key, char *buf) if (keyname != NULL) { - strcpy(buf, keyname); + TXT_StringCopy(buf, keyname, buf_len); } else if (isprint(key)) { - sprintf(buf, "%c", toupper(key)); + TXT_snprintf(buf, buf_len, "%c", toupper(key)); } else { - sprintf(buf, "??%i", key); + TXT_snprintf(buf, buf_len, "??%i", key); } } @@ -870,3 +870,62 @@ void TXT_SDL_SetEventCallback(TxtSDLEventCallbackFunc callback, void *user_data) event_callback_data = user_data; } +// Safe string functions. + +void TXT_StringCopy(char *dest, const char *src, size_t dest_len) +{ + if (dest_len < 1) + { + return; + } + + dest[dest_len - 1] = '\0'; + strncpy(dest, src, dest_len - 1); +} + +void TXT_StringConcat(char *dest, const char *src, size_t dest_len) +{ + size_t offset; + + offset = strlen(dest); + if (offset > dest_len) + { + offset = dest_len; + } + + TXT_StringCopy(dest + offset, src, dest_len - offset); +} + +// On Windows, vsnprintf() is _vsnprintf(). +#ifdef _WIN32 +#if _MSC_VER < 1400 /* not needed for Visual Studio 2008 */ +#define vsnprintf _vsnprintf +#endif +#endif + +// Safe, portable vsnprintf(). +int TXT_vsnprintf(char *buf, size_t buf_len, const char *s, va_list args) +{ + if (buf_len < 1) + { + return 0; + } + + // Windows (and other OSes?) has a vsnprintf() that doesn't always + // append a trailing \0. So we must do it, and write into a buffer + // that is one byte shorter; otherwise this function is unsafe. + buf[buf_len - 1] = '\0'; + return vsnprintf(buf, buf_len - 1, s, args); +} + +// Safe, portable snprintf(). +int TXT_snprintf(char *buf, size_t buf_len, const char *s, ...) +{ + va_list args; + int result; + va_start(args, s); + result = TXT_vsnprintf(buf, buf_len, s, args); + va_end(args); + return result; +} + diff --git a/textscreen/txt_spinctrl.c b/textscreen/txt_spinctrl.c index 1015ece5..665edb05 100644 --- a/textscreen/txt_spinctrl.c +++ b/textscreen/txt_spinctrl.c @@ -35,7 +35,7 @@ // Generate the format string to be used for displaying floats -static void FloatFormatString(float step, char *buf) +static void FloatFormatString(float step, char *buf, size_t buf_len) { int precision; @@ -43,11 +43,11 @@ static void FloatFormatString(float step, char *buf) if (precision > 0) { - sprintf(buf, "%%.%if", precision); + TXT_snprintf(buf, buf_len, "%%.%if", precision); } else { - strcpy(buf, "%.1f"); + TXT_StringCopy(buf, "%.1f", buf_len); } } @@ -57,7 +57,7 @@ static unsigned int IntWidth(int val) { char buf[25]; - sprintf(buf, "%i", val); + TXT_snprintf(buf, sizeof(buf), "%i", val); return strlen(buf); } @@ -132,12 +132,14 @@ static void SetBuffer(txt_spincontrol_t *spincontrol) switch (spincontrol->type) { case TXT_SPINCONTROL_INT: - sprintf(spincontrol->buffer, "%i", spincontrol->value->i); + TXT_snprintf(spincontrol->buffer, spincontrol->buffer_len, + "%i", spincontrol->value->i); break; case TXT_SPINCONTROL_FLOAT: - FloatFormatString(spincontrol->step.f, format); - sprintf(spincontrol->buffer, format, spincontrol->value->f); + FloatFormatString(spincontrol->step.f, format, sizeof(format)); + TXT_snprintf(spincontrol->buffer, spincontrol->buffer_len, + format, spincontrol->value->f); break; } } @@ -300,7 +302,7 @@ static int TXT_SpinControlKeyPress(TXT_UNCAST_ARG(spincontrol), int key) if (key == KEY_ENTER) { spincontrol->editing = 1; - strcpy(spincontrol->buffer, ""); + TXT_StringCopy(spincontrol->buffer, "", spincontrol->buffer_len); return 1; } if (key == KEY_LEFTARROW) @@ -387,8 +389,9 @@ static txt_spincontrol_t *TXT_BaseSpinControl(void) spincontrol = malloc(sizeof(txt_spincontrol_t)); TXT_InitWidget(spincontrol, &txt_spincontrol_class); - spincontrol->buffer = malloc(25); - strcpy(spincontrol->buffer, ""); + spincontrol->buffer_len = 25; + spincontrol->buffer = malloc(spincontrol->buffer_len); + TXT_StringCopy(spincontrol->buffer, "", spincontrol->buffer_len); spincontrol->editing = 0; return spincontrol; diff --git a/textscreen/txt_spinctrl.h b/textscreen/txt_spinctrl.h index 5a815319..006a0471 100644 --- a/textscreen/txt_spinctrl.h +++ b/textscreen/txt_spinctrl.h @@ -53,6 +53,7 @@ struct txt_spincontrol_s union { float f; int i; } min, max, *value, step; int editing; char *buffer; + size_t buffer_len; }; /** diff --git a/textscreen/txt_utf8.c b/textscreen/txt_utf8.c index 1306f265..b7b2240c 100644 --- a/textscreen/txt_utf8.c +++ b/textscreen/txt_utf8.c @@ -19,6 +19,7 @@ // 02111-1307, USA. // +#include <stdio.h> #include <stdlib.h> #include <string.h> diff --git a/textscreen/txt_utf8.h b/textscreen/txt_utf8.h index dc519032..123cae97 100644 --- a/textscreen/txt_utf8.h +++ b/textscreen/txt_utf8.h @@ -22,6 +22,8 @@ #ifndef TXT_UTF8_H #define TXT_UTF8_H +#include <stdarg.h> + char *TXT_EncodeUTF8(char *p, unsigned int c); unsigned int TXT_DecodeUTF8(const char **ptr); unsigned int TXT_UTF8_Strlen(const char *s); diff --git a/textscreen/txt_window.c b/textscreen/txt_window.c index 9ab6da9b..0eded4bd 100644 --- a/textscreen/txt_window.c +++ b/textscreen/txt_window.c @@ -511,7 +511,7 @@ txt_window_t *TXT_MessageBox(char *title, char *message, ...) va_list args; va_start(args, message); - vsnprintf(buf, sizeof(buf), message, args); + TXT_vsnprintf(buf, sizeof(buf), message, args); va_end(args); window = TXT_NewWindow(title); diff --git a/textscreen/txt_window_action.c b/textscreen/txt_window_action.c index f195f06f..8a09409b 100644 --- a/textscreen/txt_window_action.c +++ b/textscreen/txt_window_action.c @@ -35,7 +35,7 @@ static void TXT_WindowActionSizeCalc(TXT_UNCAST_ARG(action)) TXT_CAST_ARG(txt_window_action_t, action); char buf[10]; - TXT_GetKeyDescription(action->key, buf); + TXT_GetKeyDescription(action->key, buf, sizeof(buf)); // Width is label length, plus key description length, plus '=' // and two surrounding spaces. @@ -49,7 +49,7 @@ static void TXT_WindowActionDrawer(TXT_UNCAST_ARG(action)) TXT_CAST_ARG(txt_window_action_t, action); char buf[10]; - TXT_GetKeyDescription(action->key, buf); + TXT_GetKeyDescription(action->key, buf, sizeof(buf)); if (TXT_HoveringOverWidget(action)) { |