summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorrubidium <rubidium@openttd.org>2008-10-28 14:42:31 +0000
committerrubidium <rubidium@openttd.org>2008-10-28 14:42:31 +0000
commit0d254e8914f294a8f9a0e177e2d0208e57b2d5c9 (patch)
treeb5fc23e46516c5f53edc37ad11559ac6e8586d4c
parentcf4cffd91adbe563e4cd9f892ad3ab54a2fef13e (diff)
downloadopenttd-0d254e8914f294a8f9a0e177e2d0208e57b2d5c9.tar.xz
(svn r14540) -Codechange: introduce [v]seprintf which are like [v]snprintf but do return the number of characters written instead of the number of characters that would be written; as size_t is unsigned substraction can cause integer underflows quite quickly.
-rw-r--r--src/blitter/factory.hpp6
-rw-r--r--src/driver.cpp6
-rw-r--r--src/gfxinit.cpp15
-rw-r--r--src/newgrf_config.cpp2
-rw-r--r--src/openttd.cpp2
-rw-r--r--src/settings.cpp33
-rw-r--r--src/string.cpp50
-rw-r--r--src/string_func.h18
-rw-r--r--src/strings.cpp6
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;
}