libretro/scummvm
1
0
Fork 0
mirror of https://github.com/libretro/scummvm.git synced 2024-12-14 13:50:13 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

SCUMM: Fix potential bug in ScummEngine::resStrLen.

Browse Source 
In particular, it might happen that ScummEngine::resStrLen is called
while the _scriptPointer is stale. In that case, it would be working
with the stale pointer. If the code calling it then uses fetchScript*()
methods to read the string whose length was just computed, then it would
read potentially *different* data (e.g. copyScriptString or
loadPtrToResource could have been affected).

I am not sure if this actually could have caused bugs somewhere; it might
even be provable that a script relocation cannot happen in all places
that invoke resStrLen. But for now it's much easier to make the code
safe than to verify that theory ;).

Also simplified some related code.

svn-id: r53572
This commit is contained in:
Max Horn 2010-10-18 18:55:24 +00:00
parent d69a63c145
commit 6175c2bb19
3 changed files with 22 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
engines/scumm/resource.cpp
View File

@ -1012,7 +1012,7 @@ void ResourceManager::freeResources() {
void ScummEngine::loadPtrToResource(int type, int resindex, const byte *source) {
byte *alloced;
int i, len;
int len;
_res->nukeResource(type, resindex);
@ -1024,12 +1024,13 @@ void ScummEngine::loadPtrToResource(int type, int resindex, const byte *source)
alloced = _res->createResource(type, resindex, len);
if (!source) {
alloced[0] = fetchScriptByte();
for (i = 1; i < len; i++)
alloced[i] = *_scriptPointer++;
// Need to refresh the script pointer, since createResource may
// have caused the script resource to expire.
refreshScriptPointer();
memcpy(alloced, _scriptPointer, len);
_scriptPointer += len;
} else {
for (i = 0; i < len; i++)
alloced[i] = source[i];
memcpy(alloced, source, len);
}
}

25
engines/scumm/script.cpp
View File

@ -440,7 +440,6 @@ void ScummEngine::getScriptBaseAddress() {
}
}
void ScummEngine::resetScriptPointer() {
if (_currentScript == 0xFF)
return;
@ -1234,22 +1233,26 @@ bool ScummEngine::isRoomScriptRunning(int script) const {
void ScummEngine::copyScriptString(byte *dst) {
int len = resStrLen(_scriptPointer) + 1;
while (len--)
*dst++ = fetchScriptByte();
memcpy(dst, _scriptPointer, len);
_scriptPointer += len;
dst += len;
*dst = 0;
}
//
// Given a pointer to a Scumm string, this function returns the total byte length
// of the string data in that resource. To do so it has to understand certain
// special characters embedded into the string. The reason for this function is that
// sometimes this embedded data contains zero bytes, thus we can't just use strlen.
//
int ScummEngine::resStrLen(const byte *src) const {
/**
* Given a pointer to a Scumm string, this function returns the total
* byte length of the string data in that resource. To do so it has to
* understand certain special characters embedded into the string. The
* reason for this function is that sometimes this embedded data
* contains zero bytes, thus we can't just use strlen.
*/
int ScummEngine::resStrLen(const byte *src) {
int num = 0;
byte chr;
if (src == NULL)
if (src == NULL) {
refreshScriptPointer();
src = _scriptPointer;
}
while ((chr = *src++) != 0) {
num++;
if (_game.heversion <= 71 && chr == 0xFF) {

2
engines/scumm/scumm.h
View File

@ -776,7 +776,7 @@ protected:
void endOverride();
void copyScriptString(byte *dst);
int resStrLen(const byte *src) const;
int resStrLen(const byte *src);
void doSentence(int c, int b, int a);
/* Should be in Resource class */