libretro/scummvm
1
0
Fork 0
mirror of https://github.com/libretro/scummvm.git synced 2025-03-06 10:17:14 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

SCI: Fix more unsafe C-string usage

Browse Source
This commit is contained in:
Colin Snover 2017-01-05 21:05:22 -06:00
parent b1c3332fdd
commit 10d97ce379
14 changed files with 105 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
engines/sci/console.cpp
View File

@ -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] = ' ';

2
engines/sci/engine/file.cpp
View File

@ -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)

8
engines/sci/engine/kgraphics.cpp
View File

@ -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;

2
engines/sci/engine/kstring.cpp
View File

@ -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)

40
engines/sci/engine/message.cpp
View File

@ -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;
}

5
engines/sci/engine/message.h
View File

@ -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> {

8
engines/sci/engine/scriptdebug.cpp
View File

@ -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;

3
engines/sci/engine/seg_manager.cpp
View File

@ -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;

6
engines/sci/engine/segment.h
View File

@ -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);
}
}
}

11
engines/sci/graphics/controls16.cpp
View File

@ -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);

4
engines/sci/graphics/controls16.h
View File

@ -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();

27
engines/sci/graphics/text16.cpp
View File

@ -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;

10
engines/sci/graphics/text16.h
View File

@ -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;

33
engines/sci/parser/vocabulary.cpp
View File

@ -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)