Ensure null-termination in str_escape

This also fixes a couple of potential buffer overflows. The affected
code is still buggy (doesn't do the right thing on potential buffer
overflow), but at least it won't result in crashes/out of bound writes
anymore.
This commit is contained in:
heinrich5991 2017-07-08 13:06:03 +02:00
parent 33927f6397
commit 409bc0e019
6 changed files with 22 additions and 24 deletions

View file

@ -2241,12 +2241,18 @@ void str_timestamp(char *buffer, int buffer_size)
void str_escape(char **dst, const char *src, const char *end) void str_escape(char **dst, const char *src, const char *end)
{ {
while(*src && *dst < end) while(*src && *dst + 1 < end)
{ {
if(*src == '"' || *src == '\\') // escape \ and " if(*src == '"' || *src == '\\') // escape \ and "
*(*dst)++ = '\\'; {
if(*dst + 2 < end)
*(*dst)++ = '\\';
else
break;
}
*(*dst)++ = *src++; *(*dst)++ = *src++;
} }
**dst = 0;
} }
int mem_comp(const void *a, const void *b, int size) int mem_comp(const void *a, const void *b, int size)

View file

@ -1063,7 +1063,8 @@ void str_timestamp_ex(time_t time, char *buffer, int buffer_size, const char *fo
Escapes \ and " characters in a string. Escapes \ and " characters in a string.
Parameters: Parameters:
dst - Destination array pointer, gets increased dst - Destination array pointer, gets increased, will point to
the terminating null.
src - Source array src - Source array
end - End of destination array end - End of destination array
*/ */

View file

@ -167,16 +167,13 @@ void CFriends::ConfigSaveCallback(IConfig *pConfig, void *pUserData)
{ {
str_copy(aBuf, pSelf->m_Foes ? "add_foe " : "add_friend ", sizeof(aBuf)); str_copy(aBuf, pSelf->m_Foes ? "add_foe " : "add_friend ", sizeof(aBuf));
char *pDst = aBuf+str_length(aBuf); str_append(aBuf, "\"", sizeof(aBuf));
*pDst++ = '"'; char *pDst = aBuf + str_length(aBuf);
str_escape(&pDst, pSelf->m_aFriends[i].m_aName, pEnd); str_escape(&pDst, pSelf->m_aFriends[i].m_aName, pEnd);
*pDst++ = '"'; str_append(aBuf, "\" \"", sizeof(aBuf));
*pDst++ = ' '; pDst = aBuf + str_length(aBuf);
*pDst++ = '"';
str_escape(&pDst, pSelf->m_aFriends[i].m_aClan, pEnd); str_escape(&pDst, pSelf->m_aFriends[i].m_aClan, pEnd);
*pDst++ = '"'; str_append(aBuf, "\"", sizeof(aBuf));
*pDst++ = 0;
pConfig->WriteLine(aBuf); pConfig->WriteLine(aBuf);
} }

View file

@ -26,10 +26,9 @@ class CConfig : public IConfig
CCallback m_aCallbacks[MAX_CALLBACKS]; CCallback m_aCallbacks[MAX_CALLBACKS];
int m_NumCallbacks; int m_NumCallbacks;
void EscapeParam(char *pDst, const char *pSrc, int size) void EscapeParam(char *pDst, const char *pSrc, int Size)
{ {
str_escape(&pDst, pSrc, pDst + size); str_escape(&pDst, pSrc, pDst + Size);
*pDst = 0;
} }
public: public:

View file

@ -738,13 +738,10 @@ void CConsole::ConToggle(IConsole::IResult *pResult, void *pUser)
{ {
CStrVariableData *pData = static_cast<CStrVariableData *>(pUserData); CStrVariableData *pData = static_cast<CStrVariableData *>(pUserData);
const char *pStr = !str_comp(pData->m_pStr, pResult->GetString(1)) ? pResult->GetString(2) : pResult->GetString(1); const char *pStr = !str_comp(pData->m_pStr, pResult->GetString(1)) ? pResult->GetString(2) : pResult->GetString(1);
str_copy(aBuf, pResult->GetString(0), sizeof(aBuf)); str_format(aBuf, sizeof(aBuf), "%s \"", pResult->GetString(0));
char *pDst = aBuf + str_length(aBuf); char *pDst = aBuf + str_length(aBuf);
*pDst++ = ' ';
*pDst++ = '"';
str_escape(&pDst, pStr, aBuf + sizeof(aBuf)); str_escape(&pDst, pStr, aBuf + sizeof(aBuf));
*pDst++ = '"'; str_append(aBuf, "\"", sizeof(aBuf));
*pDst++ = 0;
pConsole->ExecuteLine(aBuf); pConsole->ExecuteLine(aBuf);
aBuf[0] = 0; aBuf[0] = 0;
} }

View file

@ -293,20 +293,18 @@ void CBinds::ConfigSaveCallback(IConfig *pConfig, void *pUserData)
CBinds *pSelf = (CBinds *)pUserData; CBinds *pSelf = (CBinds *)pUserData;
char aBuffer[256]; char aBuffer[256];
char *pEnd = aBuffer+sizeof(aBuffer)-8; char *pEnd = aBuffer+sizeof(aBuffer);
pConfig->WriteLine("unbindall"); pConfig->WriteLine("unbindall");
for(int i = 0; i < KEY_LAST; i++) for(int i = 0; i < KEY_LAST; i++)
{ {
if(!pSelf->m_apKeyBindings[i]) if(!pSelf->m_apKeyBindings[i])
continue; continue;
str_format(aBuffer, sizeof(aBuffer), "bind %s ", pSelf->Input()->KeyName(i)); str_format(aBuffer, sizeof(aBuffer), "bind %s \"", pSelf->Input()->KeyName(i));
// process the string. we need to escape some characters // process the string. we need to escape some characters
char *pDst = aBuffer + str_length(aBuffer); char *pDst = aBuffer + str_length(aBuffer);
*pDst++ = '"';
str_escape(&pDst, pSelf->m_apKeyBindings[i], pEnd); str_escape(&pDst, pSelf->m_apKeyBindings[i], pEnd);
*pDst++ = '"'; str_append(aBuffer, "\"", sizeof(aBuffer));
*pDst++ = 0;
pConfig->WriteLine(aBuffer); pConfig->WriteLine(aBuffer);
} }