From d3ea799f23da68077c963ef403a1dabd2ad269a7 Mon Sep 17 00:00:00 2001 From: Travis Howell Date: Wed, 13 Apr 2005 10:34:47 +0000 Subject: _stringBuffer can have mutple strings, ie in pajama2 startup. Add safety check for buffer size. svn-id: r17581 --- scumm/intern.h | 8 ++++- scumm/script_v100he.cpp | 20 ++++++------ scumm/script_v72he.cpp | 87 +++++++++++++++++++++++++++++++------------------ scumm/script_v80he.cpp | 22 ++++++------- scumm/script_v90he.cpp | 6 ++-- scumm/scumm.cpp | 12 +++++-- scumm/scumm.h | 3 -- 7 files changed, 95 insertions(+), 63 deletions(-) diff --git a/scumm/intern.h b/scumm/intern.h index 364df7fc45..75dae818be 100644 --- a/scumm/intern.h +++ b/scumm/intern.h @@ -752,11 +752,17 @@ protected: #endif const OpcodeEntryV72he *_opcodesV72he; + + int _stringLength, _stringStart; + byte _stringBuffer[4096]; + WizParameters _wizParams; public: ScummEngine_v72he(GameDetector *detector, OSystem *syst, const ScummGameSettings &gs, uint8 md5sum[16]) : ScummEngine_v70he(detector, syst, gs, md5sum) {} + virtual void scummInit(); + protected: virtual void setupOpcodes(); virtual void executeOpcode(byte i); @@ -796,7 +802,7 @@ protected: virtual void decodeParseString(int a, int b); void decodeScriptString(byte *dst, bool scriptString = false); - void copyScriptString(byte *dst); + void copyScriptString(byte *dst, int dstSize); byte *heFindResourceData(uint32 tag, byte *ptr); byte *heFindResource(uint32 tag, byte *ptr); diff --git a/scumm/script_v100he.cpp b/scumm/script_v100he.cpp index dd5b7b2428..57e876d4be 100644 --- a/scumm/script_v100he.cpp +++ b/scumm/script_v100he.cpp @@ -446,7 +446,7 @@ void ScummEngine_v100he::o100_actorOps() { debug(1,"o100_actorOps: case 32 (%d)", i); break; case 52: // SO_ACTOR_NAME - copyScriptString(string); + copyScriptString(string, sizeof(string)); loadPtrToResource(rtActorName, a->_number, string); break; case 53: // SO_ACTOR_NEW @@ -483,7 +483,7 @@ void ScummEngine_v100he::o100_actorOps() { break; case 78: { - copyScriptString(string); + copyScriptString(string, sizeof(string)); int slot = pop(); int len = resStrLen(string) + 1; @@ -596,7 +596,7 @@ void ScummEngine_v100he::o100_arrayOps() { memcpy(ah->data, string, len); break; case 77: // SO_ASSIGN_STRING - copyScriptString(string); + copyScriptString(string, sizeof(string)); len = resStrLen(string) + 1; ah = defineArray(array, kStringArray, 0, 0, 0, len); memcpy(ah->data, string, len); @@ -979,7 +979,7 @@ void ScummEngine_v100he::o100_setSpriteGroupInfo() { spriteGroupSet_inc_tx_ty(_curSpriteGroupId, value1, value2); break; case 52: - copyScriptString(string); + copyScriptString(string, sizeof(string)); break; case 53: if (!_curSpriteGroupId) @@ -1200,7 +1200,7 @@ void ScummEngine_v100he::o100_wizImageOps() { case 47: _wizParams.processFlags |= kWPFUseFile; _wizParams.processMode = 3; - copyScriptString(_wizParams.filename); + copyScriptString(_wizParams.filename, sizeof(_wizParams.filename)); break; case 53: _wizParams.processMode = 8; @@ -1235,7 +1235,7 @@ void ScummEngine_v100he::o100_wizImageOps() { case 64: _wizParams.processFlags |= kWPFUseFile; _wizParams.processMode = 4; - copyScriptString(_wizParams.filename); + copyScriptString(_wizParams.filename, sizeof(_wizParams.filename)); _wizParams.fileWriteMode = pop(); break; case 65: @@ -1272,7 +1272,7 @@ void ScummEngine_v100he::o100_wizImageOps() { pop(); pop(); pop(); - copyScriptString(filename); + copyScriptString(filename, sizeof(filename)); _wizParams.processMode = 15; break; case 129: @@ -1282,7 +1282,7 @@ void ScummEngine_v100he::o100_wizImageOps() { _wizParams.processMode = 16; pop(); pop(); - copyScriptString(filename); + copyScriptString(filename, sizeof(filename)); break; case 131: _wizParams.processMode = 13; @@ -1589,7 +1589,7 @@ void ScummEngine_v100he::o100_roomOps() { break; case 137: - copyScriptString(filename); + copyScriptString(filename, sizeof(filename)); _saveLoadFlag = pop(); _saveLoadSlot = 1; _saveTemporaryState = true; @@ -1833,7 +1833,7 @@ void ScummEngine_v100he::o100_setSpriteInfo() { spriteInfoSet_Inc_tx_ty(spriteId, args[0], args[1]); break; case 52: - copyScriptString(string); + copyScriptString(string, sizeof(string)); break; case 53: if (_curSpriteId > _curMaxSpriteId) diff --git a/scumm/script_v72he.cpp b/scumm/script_v72he.cpp index 4653a11e39..7168c03d12 100644 --- a/scumm/script_v72he.cpp +++ b/scumm/script_v72he.cpp @@ -518,20 +518,39 @@ void ScummEngine_v72he::readArrayFromIndexFile() { } } -void ScummEngine_v72he::copyScriptString(byte *dst) { - int i = 0; - byte b; +void ScummEngine_v72he::copyScriptString(byte *dst, int dstSize) { + byte string[256]; + byte chr; + int pos = 0; int array = pop(); if (array == -1) { - int len = resStrLen(_stringBuffer) + 1; + if (_stringLength == 1) + error("String stack underflow"); + + _stringLength -= 2; + while ((chr = _stringBuffer[_stringLength]) != 0) { + string[pos] = chr; + pos++; + + if (pos > dstSize) + error("String too long to pop"); + + _stringLength--; + } + + string[pos] = 0; + _stringLength++; + + // Reverse string + int len = resStrLen(string); while (len--) - *dst++ = _stringBuffer[i++]; + *dst++ = string[len]; } else { writeVar(0, array); - while ((b = readArray(0, 0, i)) != 0) { - *dst++ = b; - i++; + while ((chr = readArray(0, 0, pos)) != 0) { + *dst++ = chr; + pos++; } } *dst = 0; @@ -556,7 +575,7 @@ void ScummEngine_v72he::decodeScriptString(byte *dst, bool scriptString) { len = resStrLen(_scriptPointer); _scriptPointer += len + 1; } else { - copyScriptString(string); + copyScriptString(string, sizeof(string)); len = resStrLen(string) + 1; } @@ -719,14 +738,18 @@ void ScummEngine_v72he::o72_pushDWord() { } void ScummEngine_v72he::o72_addMessageToStack() { - _stringLength = resStrLen(_scriptPointer) + 1; - addMessageToStack(_scriptPointer, _stringBuffer, _stringLength); + byte chr; - // Filter out pointless trace messages, which often flood - if (strcmp((char *)_stringBuffer, "no trace") && strcmp((char *)_stringBuffer, "trace on")) - debug(1,"o72_addMessageToStack(\"%s\")", _scriptPointer); + while ((chr = fetchScriptByte()) != 0) { + _stringBuffer[_stringLength] = chr; + _stringLength++; + + if (_stringLength >= 4096) + error("String stack overflow"); + } - _scriptPointer += _stringLength; + _stringBuffer[_stringLength] = 0; + _stringLength++; } void ScummEngine_v72he::o72_isAnyOf() { @@ -1030,7 +1053,7 @@ void ScummEngine_v72he::o72_roomOps() { break; case 221: - copyScriptString(filename); + copyScriptString(filename, sizeof(filename)); _saveLoadFlag = pop(); _saveLoadSlot = 1; _saveTemporaryState = true; @@ -1166,7 +1189,7 @@ void ScummEngine_v72he::o72_actorOps() { a->_talkColor = pop(); break; case 88: // SO_ACTOR_NAME - copyScriptString(string); + copyScriptString(string, sizeof(string)); loadPtrToResource(rtActorName, a->_number, string); break; case 89: // SO_INIT_ANIMATION @@ -1239,7 +1262,7 @@ void ScummEngine_v72he::o72_actorOps() { break; case 225: { - copyScriptString(string); + copyScriptString(string, sizeof(string)); int slot = pop(); int len = resStrLen(string) + 1; @@ -1280,7 +1303,7 @@ void ScummEngine_v72he::o72_verbOps() { } break; case 125: // SO_VERB_NAME - copyScriptString(name); + copyScriptString(name, sizeof(name)); loadPtrToResource(rtVerb, slot, name); vs->type = kTextVerbType; vs->imgindex = 0; @@ -1394,7 +1417,7 @@ void ScummEngine_v72he::o72_arrayOps() { debug(1,"o72_arrayOps: case %d", subOp); switch (subOp) { case 7: // SO_ASSIGN_STRING - copyScriptString(string); + copyScriptString(string, sizeof(string)); len = resStrLen(string) + 1; ah = defineArray(array, kStringArray, 0, 0, 0, len); memcpy(ah->data, string, len); @@ -1590,7 +1613,7 @@ void ScummEngine_v72he::o72_dim2dimArray() { void ScummEngine_v72he::o72_traceStatus() { byte string[80]; - copyScriptString(string); + copyScriptString(string, sizeof(string)); pop(); } @@ -1624,7 +1647,7 @@ void ScummEngine_v72he::o72_drawWizImage() { void ScummEngine_v72he::o72_unknownCF() { byte string[255]; - copyScriptString(string); + copyScriptString(string, sizeof(string)); int len = resStrLen(string) + 1; writeVar(0, 0); @@ -1651,7 +1674,7 @@ void ScummEngine_v72he::o72_openFile() { byte filename[256]; mode = pop(); - copyScriptString(filename); + copyScriptString(filename, sizeof(filename)); debug(0,"Original filename %s", filename); @@ -1823,15 +1846,15 @@ void ScummEngine_v72he::o72_findAllObjects() { void ScummEngine_v72he::o72_deleteFile() { byte filename[100]; - copyScriptString(filename); + copyScriptString(filename, sizeof(filename)); debug(1, "stub o72_deleteFile(%s)", filename); } void ScummEngine_v72he::o72_rename() { byte oldFilename[256],newFilename[256]; - copyScriptString(newFilename); - copyScriptString(oldFilename); + copyScriptString(newFilename, sizeof(newFilename)); + copyScriptString(oldFilename, sizeof(oldFilename)); debug(1, "stub o72_rename(%s to %s)", oldFilename, newFilename); } @@ -2074,7 +2097,7 @@ void ScummEngine_v72he::o72_readINI() { int len, type; // we pretend that we don't have .ini file - copyScriptString(option); + copyScriptString(option, sizeof(option)); type = fetchScriptByte(); switch (type) { @@ -2111,13 +2134,13 @@ void ScummEngine_v72he::o72_writeINI() { case 43: // HE 100 case 6: // number value = pop(); - copyScriptString(option); + copyScriptString(option, sizeof(option)); ConfMan.set((char *)option, value); break; case 77: // HE 100 case 7: // string - copyScriptString(string); - copyScriptString(option); + copyScriptString(string, sizeof(string)); + copyScriptString(option, sizeof(option)); ConfMan.set((char *)option, (char *)string); break; default: @@ -2169,13 +2192,13 @@ void ScummEngine_v72he::o72_getResourceSize() { void ScummEngine_v72he::o72_setFilePath() { // File related byte filename[100]; - copyScriptString(filename); + copyScriptString(filename, sizeof(filename)); debug(1,"o72_setFilePath: %s", filename); } void ScummEngine_v72he::o72_setWindowCaption() { byte name[100]; - copyScriptString(name); + copyScriptString(name, sizeof(name)); int id = fetchScriptByte(); debug(1,"o72_setWindowCaption: (%d) %s", id, name); diff --git a/scumm/script_v80he.cpp b/scumm/script_v80he.cpp index c0413ddd0f..ba931d2ffb 100644 --- a/scumm/script_v80he.cpp +++ b/scumm/script_v80he.cpp @@ -402,7 +402,7 @@ void ScummEngine_v80he::o80_loadSBNG() { void ScummEngine_v80he::o80_getFileSize() { byte filename[256]; - copyScriptString(filename); + copyScriptString(filename, sizeof(filename)); File f; if (f.open((char *)filename) == false) { @@ -453,9 +453,9 @@ void ScummEngine_v80he::o80_readConfigFile() { int type; // we pretend that we don't have .ini file - copyScriptString(section); - copyScriptString(name); - copyScriptString(filename); + copyScriptString(section, sizeof(section)); + copyScriptString(name, sizeof(name)); + copyScriptString(filename, sizeof(filename)); type = fetchScriptByte(); switch (type) { @@ -487,17 +487,17 @@ void ScummEngine_v80he::o80_writeConfigFile() { case 43: // HE 100 case 6: // number value = pop(); - copyScriptString(section); - copyScriptString(name); - copyScriptString(filename); + copyScriptString(section, sizeof(section)); + copyScriptString(name, sizeof(name)); + copyScriptString(filename, sizeof(filename)); debug(1,"o80_writeConfigFile: Filename %s Section %s Name %s Value %d", filename, section, name, value); break; case 77: // HE 100 case 7: // string - copyScriptString(string); - copyScriptString(section); - copyScriptString(name); - copyScriptString(filename); + copyScriptString(string, sizeof(string)); + copyScriptString(section, sizeof(section)); + copyScriptString(name, sizeof(name)); + copyScriptString(filename, sizeof(filename)); debug(1,"o80_writeConfigFile: Filename %s Section %s Name %s String %s", filename, section, name, string); break; default: diff --git a/scumm/script_v90he.cpp b/scumm/script_v90he.cpp index 71e8303d31..9c93cac24d 100644 --- a/scumm/script_v90he.cpp +++ b/scumm/script_v90he.cpp @@ -541,12 +541,12 @@ void ScummEngine_v90he::o90_wizImageOps() { case 3: _wizParams.processFlags |= kWPFUseFile; _wizParams.processMode = 3; - copyScriptString(_wizParams.filename); + copyScriptString(_wizParams.filename, sizeof(_wizParams.filename)); break; case 4: _wizParams.processFlags |= kWPFUseFile; _wizParams.processMode = 4; - copyScriptString(_wizParams.filename); + copyScriptString(_wizParams.filename, sizeof(_wizParams.filename)); _wizParams.fileWriteMode = pop(); break; case 5: @@ -2285,7 +2285,7 @@ void ScummEngine_v90he::o90_unknownA5() { case 42: a = pop(); if (a == 2) { - copyScriptString(string); + copyScriptString(string, sizeof(string)); push(-1); } else if (a == 1) { pop(); diff --git a/scumm/scumm.cpp b/scumm/scumm.cpp index bacc0a1020..7dbe9abc2e 100644 --- a/scumm/scumm.cpp +++ b/scumm/scumm.cpp @@ -874,8 +874,6 @@ ScummEngine::ScummEngine(GameDetector *detector, OSystem *syst, const ScummGameS memset(_charsetData, 0, sizeof(_charsetData)); _charsetBufPos = 0; memset(_charsetBuffer, 0, sizeof(_charsetBuffer)); - _stringLength = 0; - memset(_stringBuffer, 0, sizeof(_stringBuffer)); _copyProtection = false; _demoMode = false; _confirmExit = false; @@ -1646,8 +1644,16 @@ void ScummEngine_v60he::scummInit() { setCursorHotspot(16, 16); } +void ScummEngine_v72he::scummInit() { + ScummEngine_v60he::scummInit(); + + _stringLength = 1; + _stringStart = 1; + memset(_stringBuffer, 0, sizeof(_stringBuffer)); +} + void ScummEngine_v90he::scummInit() { - ScummEngine_v80he::scummInit(); + ScummEngine_v72he::scummInit(); _heObject = 0; _heObjectNum = 0; diff --git a/scumm/scumm.h b/scumm/scumm.h index fd80ab2118..8f75b42355 100644 --- a/scumm/scumm.h +++ b/scumm/scumm.h @@ -1168,9 +1168,6 @@ protected: int _charsetBufPos; byte _charsetBuffer[512]; - int _stringLength; - byte _stringBuffer[4096]; - bool _keepText; void initCharset(int charset); -- cgit v1.2.3