Skip to content
Permalink
Browse files

Handle allocation failure in CLuaFileDefs::fileRead

Solves crash offset 0034e010
  • Loading branch information...
botder committed Oct 5, 2019
1 parent 8e9d1c6 commit 992d682756d3f79207222561a51e23e8cb43dab1
@@ -150,7 +150,7 @@ void CScriptFile::Flush()
m_pFile->FFlush();
}

long CScriptFile::Read(unsigned long ulSize, CBuffer& outBuffer)
long CScriptFile::Read(unsigned long ulSize, SString& outBuffer)
{
if (!m_pFile)
return -1;
@@ -168,8 +168,16 @@ long CScriptFile::Read(unsigned long ulSize, CBuffer& outBuffer)
// Note: Read extra byte at end so EOF indicator gets set
}

outBuffer.SetSize(ulSize);
return m_pFile->FRead(outBuffer.GetData(), ulSize);
try
{
outBuffer.resize(ulSize);
}
catch (const std::bad_alloc&)
{
return -2;
}

return m_pFile->FRead(outBuffer.data(), ulSize);
}

long CScriptFile::Write(unsigned long ulSize, const char* pData)
@@ -55,7 +55,7 @@ class CScriptFile : public CClientEntity
long SetPointer(unsigned long ulPosition);

void Flush();
long Read(unsigned long ulSize, CBuffer& outBuffer);
long Read(unsigned long ulSize, SString& outBuffer);
long Write(unsigned long ulSize, const char* pData);

// Debug info for garbage collected files
@@ -169,7 +169,7 @@ void CScriptFile::Flush()
fflush(m_pFile);
}

long CScriptFile::Read(unsigned long ulSize, CBuffer& outBuffer)
long CScriptFile::Read(unsigned long ulSize, SString& outBuffer)
{
if (!m_pFile)
return -1;
@@ -185,8 +185,16 @@ long CScriptFile::Read(unsigned long ulSize, CBuffer& outBuffer)
// Note: Read extra byte at end so EOF indicator gets set
}

outBuffer.SetSize(ulSize);
return fread(outBuffer.GetData(), 1, ulSize, m_pFile);
try
{
outBuffer.resize(ulSize);
}
catch (const std::bad_alloc&)
{
return -2;
}

return fread(outBuffer.data(), 1, ulSize, m_pFile);
}

long CScriptFile::Write(unsigned long ulSize, const char* pData)
@@ -50,7 +50,7 @@ class CScriptFile : public CElement
void SetSize(unsigned long ulNewSize);

void Flush();
long Read(unsigned long ulSize, CBuffer& outBuffer);
long Read(unsigned long ulSize, SString& outBuffer);
long Write(unsigned long ulSize, const char* pData);

// Debug info for garbage collected files
@@ -704,18 +704,24 @@ int CLuaFileDefs::fileRead(lua_State* luaVM)
}

// Allocate a buffer to read the stuff into and read some :~ into it
CBuffer buffer;

SString buffer;
long lBytesRead = pFile->Read(ulCount, buffer);
if (lBytesRead != -1)

if (lBytesRead >= 0)
{
// Push the string onto the Lua stack. Use pushlstring so we are binary
// compatible. Normal push string takes zero terminated strings.
lua_pushlstring(luaVM, buffer.GetData(), lBytesRead);
lua_pushlstring(luaVM, buffer.data(), lBytesRead);
return 1;
}

m_pScriptDebugging->LogBadPointer(luaVM, "file", 1);
else if (lBytesRead == -2)
{
m_pScriptDebugging->LogWarning(luaVM, "out of memory");
}
else
{
m_pScriptDebugging->LogBadPointer(luaVM, "file", 1);
}
}
else
m_pScriptDebugging->LogCustom(luaVM, argStream.GetFullErrorMessage());

3 comments on commit 992d682

@ccw808

This comment has been minimized.

Copy link
Member

replied Oct 5, 2019

Maybe disconnect as well? Failing to load custom models could cause desync

@botder

This comment has been minimized.

Copy link
Member Author

replied Oct 5, 2019

Model replacement is a client-side action and it is, in theory and pratice, not synced unless scripted in such way by a server. Model replacement failure must be handled by the script appropriately. We don't break backwards compatibility or behavior, because the functions are allowed to fail.

@ccw808

This comment has been minimized.

Copy link
Member

replied Oct 6, 2019

Examples of desync:
A racing server where everyone gets given a custom model except one player who gets the standard model, which could allow faster lap times.
A shooting game mode where custom models are not replaced so one player has different visibility and collisions.

Please sign in to comment.
You can’t perform that action at this time.