diff options
-rw-r--r-- | src/blitter/factory.hpp | 6 | ||||
-rw-r--r-- | src/driver.cpp | 6 | ||||
-rw-r--r-- | src/gfxinit.cpp | 15 | ||||
-rw-r--r-- | src/newgrf_config.cpp | 2 | ||||
-rw-r--r-- | src/openttd.cpp | 2 | ||||
-rw-r--r-- | src/settings.cpp | 33 | ||||
-rw-r--r-- | src/string.cpp | 50 | ||||
-rw-r--r-- | src/string_func.h | 18 | ||||
-rw-r--r-- | src/strings.cpp | 6 |
9 files changed, 97 insertions, 41 deletions
diff --git a/src/blitter/factory.hpp b/src/blitter/factory.hpp index 304d73832..30b680506 100644 --- a/src/blitter/factory.hpp +++ b/src/blitter/factory.hpp @@ -119,13 +119,13 @@ public: static char *GetBlittersInfo(char *p, const char *last) { - p += snprintf(p, last - p, "List of blitters:\n"); + p += seprintf(p, last, "List of blitters:\n"); Blitters::iterator it = GetBlitters().begin(); for (; it != GetBlitters().end(); it++) { BlitterFactoryBase *b = (*it).second; - p += snprintf(p, last - p, "%18s: %s\n", b->name, b->GetDescription()); + p += seprintf(p, last, "%18s: %s\n", b->name, b->GetDescription()); } - p += snprintf(p, last - p, "\n"); + p += seprintf(p, last, "\n"); return p; } diff --git a/src/driver.cpp b/src/driver.cpp index b042ea620..dc8ffb1af 100644 --- a/src/driver.cpp +++ b/src/driver.cpp @@ -168,7 +168,7 @@ void DriverFactoryBase::RegisterDriver(const char *name, Driver::Type type, int char *DriverFactoryBase::GetDriversInfo(char *p, const char *last) { for (Driver::Type type = Driver::DT_BEGIN; type != Driver::DT_END; type++) { - p += snprintf(p, last - p, "List of %s drivers:\n", GetDriverTypeName(type)); + p += seprintf(p, last, "List of %s drivers:\n", GetDriverTypeName(type)); for (int priority = 10; priority >= 0; priority--) { Drivers::iterator it = GetDrivers().begin(); @@ -176,11 +176,11 @@ char *DriverFactoryBase::GetDriversInfo(char *p, const char *last) DriverFactoryBase *d = (*it).second; if (d->type != type) continue; if (d->priority != priority) continue; - p += snprintf(p, last - p, "%18s: %s\n", d->name, d->GetDescription()); + p += seprintf(p, last, "%18s: %s\n", d->name, d->GetDescription()); } } - p += snprintf(p, last - p, "\n"); + p += seprintf(p, last, "\n"); } return p; diff --git a/src/gfxinit.cpp b/src/gfxinit.cpp index d63aa20b9..8d25c3d24 100644 --- a/src/gfxinit.cpp +++ b/src/gfxinit.cpp @@ -240,10 +240,11 @@ void CheckExternalFiles() char error_msg[ERROR_MESSAGE_LENGTH * (MAX_GFT + 1)]; error_msg[0] = '\0'; char *add_pos = error_msg; + const char *last = lastof(error_msg); for (uint i = 0; i < lengthof(_used_graphics_set->files); i++) { if (!FileMD5(_used_graphics_set->files[i])) { - add_pos += snprintf(add_pos, ERROR_MESSAGE_LENGTH, "Your '%s' file is corrupted or missing! %s\n", _used_graphics_set->files[i].filename, _used_graphics_set->files[i].missing_warning); + add_pos += seprintf(add_pos, last, "Your '%s' file is corrupted or missing! %s\n", _used_graphics_set->files[i].filename, _used_graphics_set->files[i].missing_warning); } } @@ -253,7 +254,7 @@ void CheckExternalFiles() } if (!sound) { - add_pos += snprintf(add_pos, ERROR_MESSAGE_LENGTH, "Your 'sample.cat' file is corrupted or missing! You can find 'sample.cat' on your Transport Tycoon Deluxe CD-ROM.\n"); + add_pos += seprintf(add_pos, last, "Your 'sample.cat' file is corrupted or missing! You can find 'sample.cat' on your Transport Tycoon Deluxe CD-ROM.\n"); } if (add_pos != error_msg) ShowInfoF(error_msg); @@ -526,19 +527,19 @@ bool SetGraphicsSet(const char *name) */ char *GetGraphicsSetsList(char *p, const char *last) { - p += snprintf(p, last - p, "List of graphics sets:\n"); + p += seprintf(p, last, "List of graphics sets:\n"); for (const GraphicsSet *g = _available_graphics_sets; g != NULL; g = g->next) { if (g->found_grfs <= 1) continue; - p += snprintf(p, last - p, "%18s: %s", g->name, g->description); + p += seprintf(p, last, "%18s: %s", g->name, g->description); int difference = MAX_GFT - g->found_grfs; if (difference != 0) { - p += snprintf(p, last - p, " (missing %i file%s)\n", difference, difference == 1 ? "" : "s"); + p += seprintf(p, last, " (missing %i file%s)\n", difference, difference == 1 ? "" : "s"); } else { - p += snprintf(p, last - p, "\n"); + p += seprintf(p, last, "\n"); } } - p += snprintf(p, last - p, "\n"); + p += seprintf(p, last, "\n"); return p; } diff --git a/src/newgrf_config.cpp b/src/newgrf_config.cpp index 235e543f8..d52ee828c 100644 --- a/src/newgrf_config.cpp +++ b/src/newgrf_config.cpp @@ -479,7 +479,7 @@ char *GRFBuildParamList(char *dst, const GRFConfig *c, const char *last) for (i = 0; i < c->num_params; i++) { if (i > 0) dst = strecpy(dst, " ", last); - dst += snprintf(dst, last - dst, "%d", c->param[i]); + dst += seprintf(dst, last, "%d", c->param[i]); } return dst; } diff --git a/src/openttd.cpp b/src/openttd.cpp index d4ef0e4fe..58d193b80 100644 --- a/src/openttd.cpp +++ b/src/openttd.cpp @@ -166,7 +166,7 @@ static void ShowHelp() char buf[4096]; char *p = buf; - p += snprintf(p, lengthof(buf), "OpenTTD %s\n", _openttd_revision); + p += seprintf(p, lastof(buf), "OpenTTD %s\n", _openttd_revision); p = strecpy(p, "\n" "\n" diff --git a/src/settings.cpp b/src/settings.cpp index 541a2c480..b3fdc36aa 100644 --- a/src/settings.cpp +++ b/src/settings.cpp @@ -211,10 +211,11 @@ static bool load_intlist(const char *str, void *array, int nelems, VarType type) /** Convert an integer-array (intlist) to a string representation. Each value * is seperated by a comma or a space character * @param buf output buffer where the string-representation will be stored + * @param last last item to write to in the output buffer * @param array pointer to the integer-arrays that is read from * @param nelems the number of elements the array holds. * @param type the type of elements the array holds (eg INT8, UINT16, etc.) */ -static void make_intlist(char *buf, const void *array, int nelems, VarType type) +static void make_intlist(char *buf, const char *last, const void *array, int nelems, VarType type) { int i, v = 0; const byte *p = (const byte*)array; @@ -230,15 +231,16 @@ static void make_intlist(char *buf, const void *array, int nelems, VarType type) case SLE_VAR_U32: v = *(uint32*)p; p += 4; break; default: NOT_REACHED(); } - buf += sprintf(buf, (i == 0) ? "%d" : ",%d", v); + buf += seprintf(buf, last, (i == 0) ? "%d" : ",%d", v); } } /** Convert a ONEofMANY structure to a string representation. * @param buf output buffer where the string-representation will be stored + * @param last last item to write to in the output buffer * @param many the full-domain string of possible values * @param id the value of the variable and whose string-representation must be found */ -static void make_oneofmany(char *buf, const char *many, int id) +static void make_oneofmany(char *buf, const char *last, const char *many, int id) { int orig_id = id; @@ -246,7 +248,7 @@ static void make_oneofmany(char *buf, const char *many, int id) while (--id >= 0) { for (; *many != '|'; many++) { if (*many == '\0') { // not found - sprintf(buf, "%d", orig_id); + seprintf(buf, last, "%d", orig_id); return; } } @@ -254,16 +256,17 @@ static void make_oneofmany(char *buf, const char *many, int id) } /* copy string until next item (|) or the end of the list if this is the last one */ - while (*many != '\0' && *many != '|') *buf++ = *many++; + while (*many != '\0' && *many != '|' && buf < last) *buf++ = *many++; *buf = '\0'; } /** Convert a MANYofMANY structure to a string representation. * @param buf output buffer where the string-representation will be stored + * @param last last item to write to in the output buffer * @param many the full-domain string of possible values * @param x the value of the variable and whose string-representation must * be found in the bitmasked many string */ -static void make_manyofmany(char *buf, const char *many, uint32 x) +static void make_manyofmany(char *buf, const char *last, const char *many, uint32 x) { const char *start; int i = 0; @@ -274,10 +277,10 @@ static void make_manyofmany(char *buf, const char *many, uint32 x) while (*many != 0 && *many != '|') many++; // advance to the next element if (HasBit(x, 0)) { // item found, copy it - if (!init) *buf++ = '|'; + if (!init) buf += seprintf(buf, last, "|"); init = false; if (start == many) { - buf += sprintf(buf, "%d", i); + buf += seprintf(buf, last, "%d", i); } else { memcpy(buf, start, many - start); buf += many - start; @@ -556,9 +559,9 @@ static void ini_save_settings(IniFile *ini, const SettingDesc *sd, const char *g switch (sdb->cmd) { case SDT_BOOLX: strcpy(buf, (i != 0) ? "true" : "false"); break; - case SDT_NUMX: sprintf(buf, IsSignedVarMemType(sld->conv) ? "%d" : "%u", i); break; - case SDT_ONEOFMANY: make_oneofmany(buf, sdb->many, i); break; - case SDT_MANYOFMANY: make_manyofmany(buf, sdb->many, i); break; + case SDT_NUMX: seprintf(buf, lastof(buf), IsSignedVarMemType(sld->conv) ? "%d" : "%u", i); break; + case SDT_ONEOFMANY: make_oneofmany(buf, lastof(buf), sdb->many, i); break; + case SDT_MANYOFMANY: make_manyofmany(buf, lastof(buf), sdb->many, i); break; default: NOT_REACHED(); } } break; @@ -566,16 +569,16 @@ static void ini_save_settings(IniFile *ini, const SettingDesc *sd, const char *g case SDT_STRING: switch (GetVarMemType(sld->conv)) { case SLE_VAR_STRB: strcpy(buf, (char*)ptr); break; - case SLE_VAR_STRBQ:sprintf(buf, "\"%s\"", (char*)ptr); break; + case SLE_VAR_STRBQ:seprintf(buf, lastof(buf), "\"%s\"", (char*)ptr); break; case SLE_VAR_STR: strcpy(buf, *(char**)ptr); break; - case SLE_VAR_STRQ: sprintf(buf, "\"%s\"", *(char**)ptr); break; - case SLE_VAR_CHAR: sprintf(buf, "\"%c\"", *(char*)ptr); break; + case SLE_VAR_STRQ: seprintf(buf, "\"%s\"", lastof(buf), *(char**)ptr); break; + case SLE_VAR_CHAR: seprintf(buf, "\"%c\"", lastof(buf), *(char*)ptr); break; default: NOT_REACHED(); } break; case SDT_INTLIST: - make_intlist(buf, ptr, sld->length, GetVarMemType(sld->conv)); + make_intlist(buf, lastof(buf), ptr, sld->length, GetVarMemType(sld->conv)); break; default: NOT_REACHED(); } diff --git a/src/string.cpp b/src/string.cpp index 9173dc4a5..3925ba24c 100644 --- a/src/string.cpp +++ b/src/string.cpp @@ -6,6 +6,7 @@ #include "openttd.h" #include "debug.h" #include "core/alloc_func.hpp" +#include "core/math_func.hpp" #include "string_func.h" #include "table/control_codes.h" @@ -59,17 +60,16 @@ char* strecpy(char* dst, const char* src, const char* last) } -char* CDECL str_fmt(const char* str, ...) +char *CDECL str_fmt(const char *str, ...) { char buf[4096]; va_list va; - int len; va_start(va, str); - len = vsnprintf(buf, lengthof(buf), str, va); + int len = vseprintf(buf, lastof(buf), str, va); va_end(va); - char* p = MallocT<char>(len + 1); - if (p != NULL) memcpy(p, buf, len + 1); + char *p = MallocT<char>(len + 1); + memcpy(p, buf, len + 1); return p; } @@ -185,6 +185,43 @@ int CDECL vsnprintf(char *str, size_t size, const char *format, va_list ap) #endif /* WIN32 */ +/** + * Safer implementation of snprintf; same as snprintf except: + * - last instead of size, i.e. replace sizeof with lastof. + * - return gives the amount of characters added, not what it would add. + * @param str buffer to write to up to last + * @param last last character we may write to + * @param format the formatting (see snprintf) + * @return the number of added characters + */ +int CDECL seprintf(char *str, const char *last, const char *format, ...) +{ + va_list ap; + + va_start(ap, format); + int ret = vseprintf(str, last, format, ap); + va_end(ap); + return ret; +} + +/** + * Safer implementation of vsnprintf; same as vsnprintf except: + * - last instead of size, i.e. replace sizeof with lastof. + * - return gives the amount of characters added, not what it would add. + * @param str buffer to write to up to last + * @param last last character we may write to + * @param format the formatting (see snprintf) + * @param ap the list of arguments for the format + * @return the number of added characters + */ +int CDECL vseprintf(char *str, const char *last, const char *format, va_list ap) +{ + if (str >= last) return 0; + size_t size = last - str; + return min((int)size, vsnprintf(str, size, format, ap)); +} + + /** Convert the md5sum to a hexadecimal string representation * @param buf buffer to put the md5sum into @@ -196,8 +233,7 @@ char *md5sumToString(char *buf, const char *last, const uint8 md5sum[16]) char *p = buf; for (uint i = 0; i < 16; i++) { - p += snprintf(p, last + 1 - p, "%02X", md5sum[i]); - if (p >= last) break; + p += seprintf(p, last, "%02X", md5sum[i]); } return p; diff --git a/src/string_func.h b/src/string_func.h index 1453170ad..c6d71b56d 100644 --- a/src/string_func.h +++ b/src/string_func.h @@ -1,6 +1,19 @@ /* $Id$ */ -/** @file string_func.h Functions related to low-level strings. */ +/** @file string_func.h Functions related to low-level strings. + * + * @note Be aware of "dangerous" string functions; string functions that + * have behaviour that could easily cause buffer overruns and such: + * - strncpy: does not '\0' terminate when input string is longer than + * the size of the output string. Use strecpy instead. + * - [v]snprintf: returns the length of the string as it would be written + * when the output is large enough, so it can be more than the size of + * the buffer and than can underflow size_t (uint-ish) which makes all + * subsequent snprintf alikes write outside of the buffer. Use + * [v]seprintf instead; it will return the number of bytes actually + * added so no [v]seprintf will cause outside of bounds writes. + * - [v]sprintf: does not bounds checking: use [v]seprintf instead. + */ #ifndef STRING_FUNC_H #define STRING_FUNC_H @@ -28,6 +41,9 @@ void ttd_strlcpy(char *dst, const char *src, size_t size); char *strecat(char *dst, const char *src, const char *last); char *strecpy(char *dst, const char *src, const char *last); +int CDECL seprintf(char *str, const char *last, const char *format, ...); +int CDECL vseprintf(char *str, const char *last, const char *format, va_list ap); + char *CDECL str_fmt(const char *str, ...); /** Scans the string for valid characters and if it finds invalid ones, diff --git a/src/strings.cpp b/src/strings.cpp index ee0ad8625..da08581a0 100644 --- a/src/strings.cpp +++ b/src/strings.cpp @@ -271,7 +271,7 @@ static char *FormatNoCommaNumber(char *buff, int64 number, const char *last) static char *FormatHexNumber(char *buff, int64 number, const char *last) { - return buff + snprintf(buff, last - buff, "0x%x", (uint32)number); + return buff + seprintf(buff, last, "0x%x", (uint32)number); } static char *FormatYmdString(char *buff, Date date, const char* last) @@ -1198,8 +1198,8 @@ static char *GetSpecialNameString(char *buff, int ind, const int64 *argv, const /* resolution size? */ if (IsInsideMM(ind, (SPECSTR_RESOLUTION_START - 0x70E4), (SPECSTR_RESOLUTION_END - 0x70E4) + 1)) { int i = ind - (SPECSTR_RESOLUTION_START - 0x70E4); - buff += snprintf( - buff, last - buff + 1, "%dx%d", _resolutions[i].width, _resolutions[i].height + buff += seprintf( + buff, last, "%dx%d", _resolutions[i].width, _resolutions[i].height ); return buff; } |