_stringBuffer can have mutple strings, ie in pajama2 startup.

Add safety check for buffer size.

svn-id: r17581

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