real_tabasco_sauce/0ad
1
0
Fork 0
forked from 0ad/0ad
Code Pull Requests Actions Activity

Fix buffer overflow in logger. Add tests for it. Make this kind of error harder to miss.

Browse Source 
This was SVN commit r7244.
This commit is contained in:
Ykkrosh 2010-01-05 19:55:09 +00:00
parent 97db62c944
commit aafe575445
3 changed files with 41 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
source/lib/sysdep/os/win/wprintf.cpp
View File

@ -208,6 +208,9 @@ int type_size(TCHAR type, int length)
int sys_vswprintf(TCHAR* buffer, size_t count, const TCHAR* format, va_list argptr)
{
// To help quickly detect incorrect 'count' values, fill the buffer with 0s
memset(buffer, 0, count*sizeof(TCHAR));
/*
Format 'variable' specifications are (in pseudo-Perl regexp syntax):

6
source/ps/CLogger.cpp
View File

@ -216,7 +216,7 @@ void CLogger::LogMessage(const wchar_t* fmt, ...)
wchar_t buffer[512];
va_start(argp, fmt);
if (sys_vswprintf(buffer, sizeof(buffer), fmt, argp) == -1)
if (sys_vswprintf(buffer, ARRAY_SIZE(buffer), fmt, argp) == -1)
{
// Buffer too small - ensure the string is nicely terminated
wcscpy_s(buffer+ARRAY_SIZE(buffer)-4, 4, L"...");
@ -232,7 +232,7 @@ void CLogger::LogWarning(const wchar_t* fmt, ...)
wchar_t buffer[512];
va_start(argp, fmt);
if (sys_vswprintf(buffer, sizeof(buffer), fmt, argp) == -1)
if (sys_vswprintf(buffer, ARRAY_SIZE(buffer), fmt, argp) == -1)
{
// Buffer too small - ensure the string is nicely terminated
wcscpy_s(buffer+ARRAY_SIZE(buffer)-4, 4, L"...");
@ -248,7 +248,7 @@ void CLogger::LogError(const wchar_t* fmt, ...)
wchar_t buffer[512];
va_start(argp, fmt);
if (sys_vswprintf(buffer, sizeof(buffer), fmt, argp) == -1)
if (sys_vswprintf(buffer, ARRAY_SIZE(buffer), fmt, argp) == -1)
{
// Buffer too small - ensure the string is nicely terminated
wcscpy_s(buffer+ARRAY_SIZE(buffer)-4, 4, L"...");

38
source/ps/tests/test_CLogger.h
View File

@ -56,16 +56,47 @@ public:
logger->LogOnce(CLogger::Normal, L"", L"%ls", msg2.c_str());
logger->LogOnce(CLogger::Normal, L"", L"%ls", msg3.c_str());
logger->LogMessage(L"%ls", msg0.c_str());
logger->LogMessage(L"%ls", msg1.c_str());
logger->LogMessage(L"%ls", msg2.c_str());
logger->LogMessage(L"%ls", msg3.c_str());
logger->LogWarning(L"%ls", msg0.c_str());
logger->LogWarning(L"%ls", msg1.c_str());
logger->LogWarning(L"%ls", msg2.c_str());
logger->LogWarning(L"%ls", msg3.c_str());
logger->LogError(L"%ls", msg0.c_str());
logger->LogError(L"%ls", msg1.c_str());
logger->LogError(L"%ls", msg2.c_str());
logger->LogError(L"%ls", msg3.c_str());
ParseOutput();
TS_ASSERT_EQUALS((int)lines.size(), 7);
TS_ASSERT_EQUALS((int)lines.size(), 4*5-1);
TS_ASSERT_EQUALS(lines[0], msg0);
TS_ASSERT_EQUALS(lines[1], msg1);
TS_ASSERT_EQUALS(lines[2], clipped);
TS_ASSERT_EQUALS(lines[3], clipped);
TS_ASSERT_EQUALS(lines[4], msg0);
TS_ASSERT_EQUALS(lines[5], msg1);
TS_ASSERT_EQUALS(lines[6], clipped);
TS_ASSERT_EQUALS(lines[7], msg0);
TS_ASSERT_EQUALS(lines[8], msg1);
TS_ASSERT_EQUALS(lines[9], clipped);
TS_ASSERT_EQUALS(lines[10], clipped);
TS_ASSERT_EQUALS(lines[11], L"WARNING: "+msg0);
TS_ASSERT_EQUALS(lines[12], L"WARNING: "+msg1);
TS_ASSERT_EQUALS(lines[13], L"WARNING: "+clipped);
TS_ASSERT_EQUALS(lines[14], L"WARNING: "+clipped);
TS_ASSERT_EQUALS(lines[15], L"ERROR: "+msg0);
TS_ASSERT_EQUALS(lines[16], L"ERROR: "+msg1);
TS_ASSERT_EQUALS(lines[17], L"ERROR: "+clipped);
TS_ASSERT_EQUALS(lines[18], L"ERROR: "+clipped);
}
//////////////////////////////////////////////////////////////////////////
@ -80,7 +111,7 @@ public:
mainlog = new std::wstringstream();
interestinglog = new std::wstringstream();
logger = new CLogger(mainlog, interestinglog, true, true);
logger = new CLogger(mainlog, interestinglog, true, false);
lines.clear();
}
@ -103,7 +134,8 @@ public:
size_t n = 0, m;
while (s.npos != (m = s.find('\n', n)))
{
lines.push_back(s.substr(n+3, m-n-7)); // strip the <p> and </p>
size_t gt = s.find('>', n);
lines.push_back(s.substr(gt+1, m-gt-5)); // strip the <p> and </p>
n = m+1;
}
}