[Scummvm-git-logs] scummvm master -> 10d97ce37947362e6fa7161cd85221f59ca49490
csnover
csnover at users.noreply.github.com
Sun Feb 5 19:47:28 CET 2017
This automated email contains information about 2 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .
Summary:
b1c3332fdd SCI: Use strnlen instead of strlen to avoid buffer overflows
10d97ce379 SCI: Fix more unsafe C-string usage
Commit: b1c3332fddbb16838f1a654d6fe35ddbe09bd051
https://github.com/scummvm/scummvm/commit/b1c3332fddbb16838f1a654d6fe35ddbe09bd051
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-02-05T10:24:02-06:00
Commit Message:
SCI: Use strnlen instead of strlen to avoid buffer overflows
Changed paths:
engines/sci/engine/kfile.cpp
engines/sci/engine/seg_manager.cpp
engines/sci/parser/vocabulary.cpp
diff --git a/engines/sci/engine/kfile.cpp b/engines/sci/engine/kfile.cpp
index 49ad4ca..25483b6 100644
--- a/engines/sci/engine/kfile.cpp
+++ b/engines/sci/engine/kfile.cpp
@@ -338,7 +338,7 @@ reg_t kFileIOOpen(EngineState *s, int argc, reg_t *argv) {
score = Common::String::format("%u%03u", save.highScore, save.lowScore);
}
- const uint nameLength = strlen(save.name);
+ const uint nameLength = Common::strnlen(save.name, SCI_MAX_SAVENAME_LENGTH);
const uint size = nameLength + /* \r\n */ 2 + score.size();
char *buffer = (char *)malloc(size);
memcpy(buffer, save.name, nameLength);
@@ -372,7 +372,7 @@ reg_t kFileIOOpen(EngineState *s, int argc, reg_t *argv) {
fillSavegameDesc(g_sci->getSavegameName(saveNo), &save);
const Common::String avatarId = Common::String::format("%02d", save.avatarId);
- const uint nameLength = strlen(save.name);
+ const uint nameLength = Common::strnlen(save.name, SCI_MAX_SAVENAME_LENGTH);
const uint size = nameLength + /* \r\n */ 2 + avatarId.size() + 1;
char *buffer = (char *)malloc(size);
memcpy(buffer, save.name, nameLength);
diff --git a/engines/sci/engine/seg_manager.cpp b/engines/sci/engine/seg_manager.cpp
index 23d1942..f8138ef 100644
--- a/engines/sci/engine/seg_manager.cpp
+++ b/engines/sci/engine/seg_manager.cpp
@@ -786,7 +786,7 @@ size_t SegManager::strlen(reg_t str) {
}
if (str_r.isRaw) {
- return ::strlen((const char *)str_r.raw);
+ return Common::strnlen((const char *)str_r.raw, str_r.maxSize);
} else {
int i = 0;
while (getChar(str_r, i))
@@ -807,19 +807,23 @@ Common::String SegManager::getString(reg_t pointer) {
return ret;
}
- if (src_r.isRaw)
- ret = (char *)src_r.raw;
- else {
+ if (src_r.isRaw) {
+ // There is no guarantee that raw strings are zero-terminated; for
+ // example, Phant1 reads "\r\n" from a pointer of size 2 during the
+ // chase
+ const uint size = Common::strnlen((const char *)src_r.raw, src_r.maxSize);
+ ret = Common::String((const char *)src_r.raw, size);
+ } else {
uint i = 0;
- for (;;) {
- char c = getChar(src_r, i);
+ while (i < (uint)src_r.maxSize) {
+ const char c = getChar(src_r, i);
if (!c)
break;
i++;
ret += c;
- };
+ }
}
return ret;
}
diff --git a/engines/sci/parser/vocabulary.cpp b/engines/sci/parser/vocabulary.cpp
index a0f9581..67197fc 100644
--- a/engines/sci/parser/vocabulary.cpp
+++ b/engines/sci/parser/vocabulary.cpp
@@ -208,7 +208,7 @@ bool Vocabulary::loadSuffixes() {
suffix_t suffix;
suffix.alt_suffix = (const char *)resource->data + seeker;
- suffix.alt_suffix_length = strlen(suffix.alt_suffix);
+ suffix.alt_suffix_length = Common::strnlen(suffix.alt_suffix, resource->size - seeker);
seeker += suffix.alt_suffix_length + 1; // Hit end of string
suffix.result_class = (int16)READ_BE_UINT16(resource->data + seeker);
@@ -218,7 +218,7 @@ bool Vocabulary::loadSuffixes() {
seeker++;
suffix.word_suffix = (const char *)resource->data + seeker;
- suffix.word_suffix_length = strlen(suffix.word_suffix);
+ suffix.word_suffix_length = Common::strnlen(suffix.word_suffix, resource->size - seeker);
seeker += suffix.word_suffix_length + 1;
suffix.class_mask = (int16)READ_BE_UINT16(resource->data + seeker);
@@ -288,12 +288,12 @@ bool Vocabulary::loadAltInputs() {
AltInput t;
t._input = data;
- uint32 l = strlen(data);
+ uint32 l = Common::strnlen(data, data_end - data);
t._inputLength = l;
data += l + 1;
t._replacement = data;
- l = strlen(data);
+ l = Common::strnlen(data, data_end - data);
data += l + 1;
if (data < data_end && strncmp(data, t._input, t._inputLength) == 0)
Commit: 10d97ce37947362e6fa7161cd85221f59ca49490
https://github.com/scummvm/scummvm/commit/10d97ce37947362e6fa7161cd85221f59ca49490
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-02-05T12:38:21-06:00
Commit Message:
SCI: Fix more unsafe C-string usage
Changed paths:
engines/sci/console.cpp
engines/sci/engine/file.cpp
engines/sci/engine/kgraphics.cpp
engines/sci/engine/kstring.cpp
engines/sci/engine/message.cpp
engines/sci/engine/message.h
engines/sci/engine/scriptdebug.cpp
engines/sci/engine/seg_manager.cpp
engines/sci/engine/segment.h
engines/sci/graphics/controls16.cpp
engines/sci/graphics/controls16.h
engines/sci/graphics/text16.cpp
engines/sci/graphics/text16.h
engines/sci/parser/vocabulary.cpp
diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index f1bb8d3..ae8ab14 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -1264,7 +1264,7 @@ bool Console::cmdMapInstrument(int argc, const char **argv) {
char *instrumentName = new char[11];
Common::strlcpy(instrumentName, argv[1], 11);
- for (uint16 i = 0; i < strlen(instrumentName); i++)
+ for (uint16 i = 0; i < Common::strnlen(instrumentName, 11); i++)
if (instrumentName[i] == '_')
instrumentName[i] = ' ';
diff --git a/engines/sci/engine/file.cpp b/engines/sci/engine/file.cpp
index f4bd437d..91cf189 100644
--- a/engines/sci/engine/file.cpp
+++ b/engines/sci/engine/file.cpp
@@ -313,7 +313,7 @@ int fgets_wrapper(EngineState *s, char *dest, int maxsize, int handle) {
if (maxsize > 1) {
memset(dest, 0, maxsize);
f->_in->readLine(dest, maxsize);
- readBytes = strlen(dest); // FIXME: sierra sci returned byte count and didn't react on NUL characters
+ readBytes = Common::strnlen(dest, maxsize); // FIXME: sierra sci returned byte count and didn't react on NUL characters
// The returned string must not have an ending LF
if (readBytes > 0) {
if (dest[readBytes - 1] == 0x0A)
diff --git a/engines/sci/engine/kgraphics.cpp b/engines/sci/engine/kgraphics.cpp
index 08e3115..07a1c47 100644
--- a/engines/sci/engine/kgraphics.cpp
+++ b/engines/sci/engine/kgraphics.cpp
@@ -824,8 +824,7 @@ void _k_GenericDrawControl(EngineState *s, reg_t controlObject, bool hilite) {
int16 celNo;
int16 priority;
reg_t listSeeker;
- Common::String *listStrings = NULL;
- const char **listEntries = NULL;
+ Common::String *listStrings = nullptr;
bool isAlias = false;
rect = kControlCreateRect(x, y,
@@ -922,11 +921,9 @@ void _k_GenericDrawControl(EngineState *s, reg_t controlObject, bool hilite) {
if (listCount) {
// We create a pointer-list to the different strings, we also find out whats upper and cursor position
listSeeker = textReference;
- listEntries = (const char**)malloc(sizeof(char *) * listCount);
listStrings = new Common::String[listCount];
for (i = 0; i < listCount; i++) {
listStrings[i] = s->_segMan->getString(listSeeker);
- listEntries[i] = listStrings[i].c_str();
if (listSeeker.getOffset() == upperOffset)
upperPos = i;
if (listSeeker.getOffset() == cursorOffset)
@@ -936,8 +933,7 @@ void _k_GenericDrawControl(EngineState *s, reg_t controlObject, bool hilite) {
}
debugC(kDebugLevelGraphics, "drawing list control %04x:%04x to %d,%d, diff %d", PRINT_REG(controlObject), x, y, SCI_MAX_SAVENAME_LENGTH);
- g_sci->_gfxControls16->kernelDrawList(rect, controlObject, maxChars, listCount, listEntries, fontId, style, upperPos, cursorPos, isAlias, hilite);
- free(listEntries);
+ g_sci->_gfxControls16->kernelDrawList(rect, controlObject, maxChars, listCount, listStrings, fontId, style, upperPos, cursorPos, isAlias, hilite);
delete[] listStrings;
return;
diff --git a/engines/sci/engine/kstring.cpp b/engines/sci/engine/kstring.cpp
index ae91ef0..4c5f122 100644
--- a/engines/sci/engine/kstring.cpp
+++ b/engines/sci/engine/kstring.cpp
@@ -305,7 +305,7 @@ reg_t kFormat(EngineState *s, int argc, reg_t *argv) {
Common::String tempsource = g_sci->getKernel()->lookupText(reg,
arguments[paramindex + 1]);
- int slen = strlen(tempsource.c_str());
+ int slen = tempsource.size();
int extralen = strLength - slen;
assert((target - targetbuf) + extralen <= maxsize);
if (extralen < 0)
diff --git a/engines/sci/engine/message.cpp b/engines/sci/engine/message.cpp
index 26ab9b4..5e07ead 100644
--- a/engines/sci/engine/message.cpp
+++ b/engines/sci/engine/message.cpp
@@ -32,6 +32,7 @@ struct MessageRecord {
MessageTuple tuple;
MessageTuple refTuple;
const char *string;
+ uint32 length;
byte talker;
};
@@ -77,7 +78,13 @@ public:
record.tuple = tuple;
record.refTuple = MessageTuple();
record.talker = 0;
- record.string = (const char *)_data + READ_LE_UINT16(recordPtr + 2);
+ const uint16 stringOffset = READ_LE_UINT16(recordPtr + 2);
+ const uint32 maxSize = _size - stringOffset;
+ record.string = (const char *)_data + stringOffset;
+ record.length = Common::strnlen(record.string, maxSize);
+ if (record.length == maxSize) {
+ warning("Message %s appears truncated at %ld", tuple.toString().c_str(), recordPtr - _data);
+ }
return true;
}
recordPtr += _recordSize;
@@ -100,7 +107,13 @@ public:
record.tuple = tuple;
record.refTuple = MessageTuple();
record.talker = recordPtr[4];
- record.string = (const char *)_data + READ_LE_UINT16(recordPtr + 5);
+ const uint16 stringOffset = READ_LE_UINT16(recordPtr + 5);
+ const uint32 maxSize = _size - stringOffset;
+ record.string = (const char *)_data + stringOffset;
+ record.length = Common::strnlen(record.string, maxSize);
+ if (record.length == maxSize) {
+ warning("Message %s appears truncated at %ld", tuple.toString().c_str(), recordPtr - _data);
+ }
return true;
}
recordPtr += _recordSize;
@@ -123,7 +136,13 @@ public:
record.tuple = tuple;
record.refTuple = MessageTuple(recordPtr[7], recordPtr[8], recordPtr[9]);
record.talker = recordPtr[4];
- record.string = (const char *)_data + READ_SCI11ENDIAN_UINT16(recordPtr + 5);
+ const uint16 stringOffset = READ_SCI11ENDIAN_UINT16(recordPtr + 5);
+ const uint32 maxSize = _size - stringOffset;
+ record.string = (const char *)_data + stringOffset;
+ record.length = Common::strnlen(record.string, maxSize);
+ if (record.length == maxSize) {
+ warning("Message %s appears truncated at %ld", tuple.toString().c_str(), recordPtr - _data);
+ }
return true;
}
recordPtr += _recordSize;
@@ -149,7 +168,13 @@ public:
record.tuple = tuple;
record.refTuple = MessageTuple(recordPtr[8], recordPtr[9], recordPtr[10]);
record.talker = recordPtr[4];
- record.string = (const char *)_data + READ_BE_UINT16(recordPtr + 6);
+ const uint16 stringOffset = READ_BE_UINT16(recordPtr + 6);
+ const uint32 maxSize = _size - stringOffset;
+ record.string = (const char *)_data + stringOffset;
+ record.length = Common::strnlen(record.string, maxSize);
+ if (record.length == maxSize) {
+ warning("Message %s appears truncated at %ld", tuple.toString().c_str(), recordPtr - _data);
+ }
return true;
}
recordPtr += _recordSize;
@@ -161,7 +186,7 @@ public:
#endif
bool MessageState::getRecord(CursorStack &stack, bool recurse, MessageRecord &record) {
- Resource *res = g_sci->getResMan()->findResource(ResourceId(kResourceTypeMessage, stack.getModule()), 0);
+ Resource *res = g_sci->getResMan()->findResource(ResourceId(kResourceTypeMessage, stack.getModule()), false);
if (!res) {
warning("Failed to open message resource %d", stack.getModule());
@@ -238,6 +263,7 @@ bool MessageState::getRecord(CursorStack &stack, bool recurse, MessageRecord &re
// as the text shown in this screen is very short (one-liners).
// Just output an empty string here instead of showing an error.
record.string = "";
+ record.length = 0;
delete reader;
return true;
}
@@ -285,7 +311,7 @@ int MessageState::nextMessage(reg_t buf) {
return record.talker;
} else {
MessageTuple &t = _cursorStack.top();
- outputString(buf, Common::String::format("Msg %d: %d %d %d %d not found", _cursorStack.getModule(), t.noun, t.verb, t.cond, t.seq));
+ outputString(buf, Common::String::format("Msg %d: %s not found", _cursorStack.getModule(), t.toString().c_str()));
return 0;
}
} else {
@@ -304,7 +330,7 @@ int MessageState::messageSize(int module, MessageTuple &t) {
stack.init(module, t);
if (getRecord(stack, true, record))
- return strlen(record.string) + 1;
+ return record.length + 1;
else
return 0;
}
diff --git a/engines/sci/engine/message.h b/engines/sci/engine/message.h
index 5847e47..a4de19b 100644
--- a/engines/sci/engine/message.h
+++ b/engines/sci/engine/message.h
@@ -40,6 +40,11 @@ struct MessageTuple {
MessageTuple(byte noun_ = 0, byte verb_ = 0, byte cond_ = 0, byte seq_ = 1)
: noun(noun_), verb(verb_), cond(cond_), seq(seq_) { }
+
+ Common::String toString() const {
+ return Common::String::format("noun %d, verb %d, cond %d, seq %d",
+ noun, verb, cond, seq);
+ }
};
class CursorStack : public Common::Stack<MessageTuple> {
diff --git a/engines/sci/engine/scriptdebug.cpp b/engines/sci/engine/scriptdebug.cpp
index 54b925a..6002cbd 100644
--- a/engines/sci/engine/scriptdebug.cpp
+++ b/engines/sci/engine/scriptdebug.cpp
@@ -598,8 +598,12 @@ void Kernel::dissectScript(int scriptNumber, Vocabulary *vocab) {
case SCI_OBJ_STRINGS:
debugN("Strings\n");
while (script->data [seeker]) {
- debugN("%04x: %s\n", seeker, script->data + seeker);
- seeker += strlen((char *)script->data + seeker) + 1;
+ debugN("%04x: %s", seeker, script->data + seeker);
+ seeker += Common::strnlen((char *)script->data + seeker, script->size - seeker) + 1;
+ if (seeker > script->size) {
+ debugN("[TRUNCATED]");
+ }
+ debugN("\n");
}
seeker++; // the ending zero byte
break;
diff --git a/engines/sci/engine/seg_manager.cpp b/engines/sci/engine/seg_manager.cpp
index f8138ef..9ccd109 100644
--- a/engines/sci/engine/seg_manager.cpp
+++ b/engines/sci/engine/seg_manager.cpp
@@ -786,6 +786,9 @@ size_t SegManager::strlen(reg_t str) {
}
if (str_r.isRaw) {
+ // There is no guarantee that raw strings are zero-terminated; for
+ // example, Phant1 reads "\r\n" from a pointer of size 2 during the
+ // chase
return Common::strnlen((const char *)str_r.raw, str_r.maxSize);
} else {
int i = 0;
diff --git a/engines/sci/engine/segment.h b/engines/sci/engine/segment.h
index 1ec1317..281837d 100644
--- a/engines/sci/engine/segment.h
+++ b/engines/sci/engine/segment.h
@@ -528,7 +528,7 @@ public:
*/
void snug() {
assert(_type == kArrayTypeString || _type == kArrayTypeByte);
- resize(strlen((char *)_data) + 1, true);
+ resize(Common::strnlen((char *)_data, _size) + 1, true);
}
/**
@@ -808,7 +808,7 @@ public:
}
if (flags & kArrayTrimRight) {
- source = data + strlen((char *)data) - 1;
+ source = data + Common::strnlen((char *)data, _size) - 1;
while (source > data && *source != showChar && *source <= kWhitespaceBoundary) {
*source = '\0';
--source;
@@ -844,7 +844,7 @@ public:
}
++source;
- memmove(target, source, strlen((char *)source) + 1);
+ memmove(target, source, Common::strnlen((char *)source, _size - 1) + 1);
}
}
}
diff --git a/engines/sci/graphics/controls16.cpp b/engines/sci/graphics/controls16.cpp
index b4bd926..479044a 100644
--- a/engines/sci/graphics/controls16.cpp
+++ b/engines/sci/graphics/controls16.cpp
@@ -52,14 +52,12 @@ GfxControls16::~GfxControls16() {
const char controlListUpArrow[2] = { 0x18, 0 };
const char controlListDownArrow[2] = { 0x19, 0 };
-void GfxControls16::drawListControl(Common::Rect rect, reg_t obj, int16 maxChars, int16 count, const char **entries, GuiResourceId fontId, int16 upperPos, int16 cursorPos, bool isAlias) {
+void GfxControls16::drawListControl(Common::Rect rect, reg_t obj, int16 maxChars, int16 count, const Common::String *entries, GuiResourceId fontId, int16 upperPos, int16 cursorPos, bool isAlias) {
Common::Rect workerRect = rect;
GuiResourceId oldFontId = _text16->GetFontId();
int16 oldPenColor = _ports->_curPort->penClr;
uint16 fontSize = 0;
int16 i;
- const char *listEntry;
- int16 listEntryLen;
int16 lastYpos;
// draw basic window
@@ -92,11 +90,10 @@ void GfxControls16::drawListControl(Common::Rect rect, reg_t obj, int16 maxChars
// Write actual text
for (i = upperPos; i < count; i++) {
_paint16->eraseRect(workerRect);
- listEntry = entries[i];
+ const Common::String &listEntry = entries[i];
if (listEntry[0]) {
_ports->moveTo(workerRect.left, workerRect.top);
- listEntryLen = strlen(listEntry);
- _text16->Draw(listEntry, 0, MIN(maxChars, listEntryLen), oldFontId, oldPenColor);
+ _text16->Draw(listEntry.c_str(), 0, MIN<int16>(maxChars, listEntry.size()), oldFontId, oldPenColor);
if ((!isAlias) && (i == cursorPos)) {
_paint16->invertRect(workerRect);
}
@@ -370,7 +367,7 @@ void GfxControls16::kernelDrawIcon(Common::Rect rect, reg_t obj, GuiResourceId v
}
}
-void GfxControls16::kernelDrawList(Common::Rect rect, reg_t obj, int16 maxChars, int16 count, const char **entries, GuiResourceId fontId, int16 style, int16 upperPos, int16 cursorPos, bool isAlias, bool hilite) {
+void GfxControls16::kernelDrawList(Common::Rect rect, reg_t obj, int16 maxChars, int16 count, const Common::String *entries, GuiResourceId fontId, int16 style, int16 upperPos, int16 cursorPos, bool isAlias, bool hilite) {
if (!hilite) {
drawListControl(rect, obj, maxChars, count, entries, fontId, upperPos, cursorPos, isAlias);
rect.grow(1);
diff --git a/engines/sci/graphics/controls16.h b/engines/sci/graphics/controls16.h
index 39ffa24..09d5800 100644
--- a/engines/sci/graphics/controls16.h
+++ b/engines/sci/graphics/controls16.h
@@ -59,13 +59,13 @@ public:
void kernelDrawText(Common::Rect rect, reg_t obj, const char *text, uint16 languageSplitter, int16 fontId, int16 alignment, int16 style, bool hilite);
void kernelDrawTextEdit(Common::Rect rect, reg_t obj, const char *text, uint16 languageSplitter, int16 fontId, int16 mode, int16 style, int16 cursorPos, int16 maxChars, bool hilite);
void kernelDrawIcon(Common::Rect rect, reg_t obj, GuiResourceId viewId, int16 loopNo, int16 celNo, int16 priority, int16 style, bool hilite);
- void kernelDrawList(Common::Rect rect, reg_t obj, int16 maxChars, int16 count, const char **entries, GuiResourceId fontId, int16 style, int16 upperPos, int16 cursorPos, bool isAlias, bool hilite);
+ void kernelDrawList(Common::Rect rect, reg_t obj, int16 maxChars, int16 count, const Common::String *entries, GuiResourceId fontId, int16 style, int16 upperPos, int16 cursorPos, bool isAlias, bool hilite);
void kernelTexteditChange(reg_t controlObject, reg_t eventObject);
private:
void texteditSetBlinkTime();
- void drawListControl(Common::Rect rect, reg_t obj, int16 maxChars, int16 count, const char **entries, GuiResourceId fontId, int16 upperPos, int16 cursorPos, bool isAlias);
+ void drawListControl(Common::Rect rect, reg_t obj, int16 maxChars, int16 count, const Common::String *entries, GuiResourceId fontId, int16 upperPos, int16 cursorPos, bool isAlias);
void texteditCursorDraw(Common::Rect rect, const char *text, uint16 curPos);
void texteditCursorErase();
int getPicNotValid();
diff --git a/engines/sci/graphics/text16.cpp b/engines/sci/graphics/text16.cpp
index b5dd9ae..903136c 100644
--- a/engines/sci/graphics/text16.cpp
+++ b/engines/sci/graphics/text16.cpp
@@ -200,7 +200,7 @@ int16 GfxText16::GetLongest(const char *&textPtr, int16 maxWidth, GuiResourceId
if (!_font)
return 0;
- while (1) {
+ for (;;) {
curChar = (*(const byte *)textPtr);
if (_font->isDoubleByte(curChar)) {
curChar |= (*(const byte *)(textPtr + 1)) << 8;
@@ -300,7 +300,7 @@ int16 GfxText16::GetLongest(const char *&textPtr, int16 maxWidth, GuiResourceId
punctuationTable = text16_shiftJIS_punctuation_SCI01;
}
- while (1) {
+ for (;;) {
// Look up if character shouldn't be the first on a new line
nonBreakingPos = 0;
while (punctuationTable[nonBreakingPos]) {
@@ -383,15 +383,15 @@ void GfxText16::Width(const char *text, int16 from, int16 len, GuiResourceId org
return;
}
-void GfxText16::StringWidth(const char *str, GuiResourceId orgFontId, int16 &textWidth, int16 &textHeight) {
- Width(str, 0, (int16)strlen(str), orgFontId, textWidth, textHeight, true);
+void GfxText16::StringWidth(const Common::String &str, GuiResourceId orgFontId, int16 &textWidth, int16 &textHeight) {
+ Width(str.c_str(), 0, str.size(), orgFontId, textWidth, textHeight, true);
}
-void GfxText16::ShowString(const char *str, GuiResourceId orgFontId, int16 orgPenColor) {
- Show(str, 0, (int16)strlen(str), orgFontId, orgPenColor);
+void GfxText16::ShowString(const Common::String &str, GuiResourceId orgFontId, int16 orgPenColor) {
+ Show(str.c_str(), 0, str.size(), orgFontId, orgPenColor);
}
-void GfxText16::DrawString(const char *str, GuiResourceId orgFontId, int16 orgPenColor) {
- Draw(str, 0, (int16)strlen(str), orgFontId, orgPenColor);
+void GfxText16::DrawString(const Common::String &str, GuiResourceId orgFontId, int16 orgPenColor) {
+ Draw(str.c_str(), 0, str.size(), orgFontId, orgPenColor);
}
int16 GfxText16::Size(Common::Rect &rect, const char *text, uint16 languageSplitter, GuiResourceId fontId, int16 maxWidth) {
@@ -580,20 +580,21 @@ void GfxText16::Box(const char *text, uint16 languageSplitter, bool show, const
}
}
-void GfxText16::DrawString(const char *text) {
+void GfxText16::DrawString(const Common::String &text) {
GuiResourceId previousFontId = GetFontId();
int16 previousPenColor = _ports->_curPort->penClr;
- Draw(text, 0, strlen(text), previousFontId, previousPenColor);
+ Draw(text.c_str(), 0, text.size(), previousFontId, previousPenColor);
SetFont(previousFontId);
_ports->penColor(previousPenColor);
}
// we need to have a separate status drawing code
// In KQ4 the IV char is actually 0xA, which would otherwise get considered as linebreak and not printed
-void GfxText16::DrawStatus(const char *text) {
+void GfxText16::DrawStatus(const Common::String &str) {
uint16 curChar, charWidth;
- uint16 textLen = strlen(text);
+ const byte *text = (const byte *)str.c_str();
+ uint16 textLen = str.size();
Common::Rect rect;
GetFont();
@@ -603,7 +604,7 @@ void GfxText16::DrawStatus(const char *text) {
rect.top = _ports->_curPort->curTop;
rect.bottom = rect.top + _ports->_curPort->fontHeight;
while (textLen--) {
- curChar = (*(const byte *)text++);
+ curChar = *text++;
switch (curChar) {
case 0:
break;
diff --git a/engines/sci/graphics/text16.h b/engines/sci/graphics/text16.h
index eb39fb2..cb3deb0 100644
--- a/engines/sci/graphics/text16.h
+++ b/engines/sci/graphics/text16.h
@@ -53,9 +53,9 @@ public:
int16 GetLongest(const char *&text, int16 maxWidth, GuiResourceId orgFontId);
void Width(const char *text, int16 from, int16 len, GuiResourceId orgFontId, int16 &textWidth, int16 &textHeight, bool restoreFont);
- void StringWidth(const char *str, GuiResourceId orgFontId, int16 &textWidth, int16 &textHeight);
- void ShowString(const char *str, GuiResourceId orgFontId, int16 orgPenColor);
- void DrawString(const char *str, GuiResourceId orgFontId, int16 orgPenColor);
+ void StringWidth(const Common::String &str, GuiResourceId orgFontId, int16 &textWidth, int16 &textHeight);
+ void ShowString(const Common::String &str, GuiResourceId orgFontId, int16 orgPenColor);
+ void DrawString(const Common::String &str, GuiResourceId orgFontId, int16 orgPenColor);
int16 Size(Common::Rect &rect, const char *text, uint16 textLanguage, GuiResourceId fontId, int16 maxWidth);
void Draw(const char *text, int16 from, int16 len, GuiResourceId orgFontId, int16 orgPenColor);
void Show(const char *text, int16 from, int16 len, GuiResourceId orgFontId, int16 orgPenColor);
@@ -65,8 +65,8 @@ public:
Box(text, 0, show, rect, alignment, fontId);
}
- void DrawString(const char *text);
- void DrawStatus(const char *text);
+ void DrawString(const Common::String &str);
+ void DrawStatus(const Common::String &str);
GfxFont *_font;
diff --git a/engines/sci/parser/vocabulary.cpp b/engines/sci/parser/vocabulary.cpp
index 67197fc..6b90cd8 100644
--- a/engines/sci/parser/vocabulary.cpp
+++ b/engines/sci/parser/vocabulary.cpp
@@ -207,8 +207,12 @@ bool Vocabulary::loadSuffixes() {
while ((seeker < resource->size - 1) && (resource->data[seeker + 1] != 0xff)) {
suffix_t suffix;
+ int maxSize = resource->size - seeker;
suffix.alt_suffix = (const char *)resource->data + seeker;
- suffix.alt_suffix_length = Common::strnlen(suffix.alt_suffix, resource->size - seeker);
+ suffix.alt_suffix_length = Common::strnlen(suffix.alt_suffix, maxSize);
+ if (suffix.alt_suffix_length == maxSize) {
+ error("Vocabulary alt appears truncated for suffix %d in resource %d at %u", _parserSuffixes.size(), resource->getNumber(), seeker);
+ }
seeker += suffix.alt_suffix_length + 1; // Hit end of string
suffix.result_class = (int16)READ_BE_UINT16(resource->data + seeker);
@@ -217,8 +221,12 @@ bool Vocabulary::loadSuffixes() {
// Beginning of next string - skip leading '*'
seeker++;
+ maxSize = resource->size - seeker;
suffix.word_suffix = (const char *)resource->data + seeker;
- suffix.word_suffix_length = Common::strnlen(suffix.word_suffix, resource->size - seeker);
+ suffix.word_suffix_length = Common::strnlen(suffix.word_suffix, maxSize);
+ if (suffix.word_suffix_length == maxSize) {
+ error("Vocabulary word appears truncated for suffix %d in resource %d at %u", _parserSuffixes.size(), resource->getNumber(), seeker);
+ }
seeker += suffix.word_suffix_length + 1;
suffix.class_mask = (int16)READ_BE_UINT16(resource->data + seeker);
@@ -288,12 +296,20 @@ bool Vocabulary::loadAltInputs() {
AltInput t;
t._input = data;
- uint32 l = Common::strnlen(data, data_end - data);
+ uint32 maxSize = data_end - data;
+ uint32 l = Common::strnlen(data, maxSize);
+ if (l == maxSize) {
+ error("Alt input from %d appears truncated at %ld", resource->getNumber(), (byte *)data - resource->data);
+ }
t._inputLength = l;
data += l + 1;
t._replacement = data;
- l = Common::strnlen(data, data_end - data);
+ maxSize = data_end - data;
+ l = Common::strnlen(data, maxSize);
+ if (l == maxSize) {
+ error("Alt input replacement from %d appears truncated at %ld", resource->getNumber(), (byte *)data - resource->data);
+ }
data += l + 1;
if (data < data_end && strncmp(data, t._input, t._inputLength) == 0)
@@ -316,7 +332,7 @@ void Vocabulary::freeAltInputs() {
_altInputs.clear();
}
-bool Vocabulary::checkAltInput(Common::String& text, uint16& cursorPos) {
+bool Vocabulary::checkAltInput(Common::String &text, uint16 &cursorPos) {
if (_altInputs.empty())
return false;
if (SELECTOR(parseLang) == -1)
@@ -330,7 +346,7 @@ bool Vocabulary::checkAltInput(Common::String& text, uint16& cursorPos) {
do {
changed = false;
- const char* t = text.c_str();
+ const char *t = text.c_str();
uint32 tlen = text.size();
for (uint32 p = 0; p < tlen && !changed; ++p) {
@@ -345,10 +361,11 @@ bool Vocabulary::checkAltInput(Common::String& text, uint16& cursorPos) {
continue;
if (strncmp(i->_input, t+p, i->_inputLength) == 0) {
// replace
+ const uint32 maxSize = text.size() - cursorPos;
if (cursorPos > p + i->_inputLength) {
- cursorPos += strlen(i->_replacement) - i->_inputLength;
+ cursorPos += Common::strnlen(i->_replacement, maxSize) - i->_inputLength;
} else if (cursorPos > p) {
- cursorPos = p + strlen(i->_replacement);
+ cursorPos = p + Common::strnlen(i->_replacement, maxSize);
}
for (uint32 j = 0; j < i->_inputLength; ++j)
More information about the Scummvm-git-logs
mailing list