From b55758fd0b70c9dfff3dfa6e14c3351ee4da5cfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herm=C3=A8s=20B=C3=A9lusca-Ma=C3=AFto?= Date: Sat, 11 Nov 2023 17:21:40 +0100 Subject: [PATCH] [CRT] dbgrpt.cpp: Systematically use CRT *_s() "secure" string functions (#5678) --- sdk/lib/crt/misc/dbgrpt.cpp | 88 +++++++++++++++++++++++-------------- 1 file changed, 54 insertions(+), 34 deletions(-) diff --git a/sdk/lib/crt/misc/dbgrpt.cpp b/sdk/lib/crt/misc/dbgrpt.cpp index 51c55af9b4c5b..4d9ef0bdb2455 100644 --- a/sdk/lib/crt/misc/dbgrpt.cpp +++ b/sdk/lib/crt/misc/dbgrpt.cpp @@ -133,7 +133,7 @@ HMODULE _CrtGetUser32() } } - return _CrtUser32Handle != INVALID_HANDLE_VALUE ? _CrtUser32Handle : NULL; + return (_CrtUser32Handle != INVALID_HANDLE_VALUE ? _CrtUser32Handle : NULL); } static tMessageBoxW _CrtGetMessageBox() @@ -149,7 +149,7 @@ static tMessageBoxW _CrtGetMessageBox() _InterlockedCompareExchangePointer((PVOID*)&_CrtMessageBoxW, (PVOID)proc, NULL); } - return _CrtMessageBoxW != INVALID_HANDLE_VALUE ? _CrtMessageBoxW : NULL; + return (_CrtMessageBoxW != INVALID_HANDLE_VALUE ? _CrtMessageBoxW : NULL); } @@ -158,7 +158,7 @@ static int _CrtDbgReportWindow(int reportType, const char_t *filename, int linen { typedef dbgrpt_char_traits traits; - wchar_t szCompleteMessage[(DBGRPT_MAX_BUFFER_SIZE+1)*2] = {0}; + wchar_t szCompleteMessage[DBGRPT_MAX_BUFFER_SIZE] = {0}; wchar_t LineBuffer[20] = {0}; if (filename && !filename[0]) @@ -168,15 +168,17 @@ static int _CrtDbgReportWindow(int reportType, const char_t *filename, int linen if (message && !message[0]) message = NULL; if (linenumber) - _itow(linenumber, LineBuffer, 10); - - _snwprintf(szCompleteMessage, DBGRPT_MAX_BUFFER_SIZE * 2, - traits::szAssertionMessage, - _CrtModeMessages[reportType], - moduleName ? L"\nModule: " : L"", moduleName ? moduleName : traits::szEmptyString, - filename ? L"\nFile: " : L"", filename ? filename : traits::szEmptyString, - LineBuffer[0] ? L"\nLine: " : L"", LineBuffer[0] ? LineBuffer : L"", - message ? L"\n\n" : L"", message ? message : traits::szEmptyString); + _itow_s(linenumber, LineBuffer, _countof(LineBuffer), 10); + + _snwprintf_s(szCompleteMessage, + _countof(szCompleteMessage), + _countof(szCompleteMessage) - 1, + traits::szAssertionMessage, + _CrtModeMessages[reportType], + moduleName ? L"\nModule: " : L"", moduleName ? moduleName : traits::szEmptyString, + filename ? L"\nFile: " : L"", filename ? filename : traits::szEmptyString, + LineBuffer[0] ? L"\nLine: " : L"", LineBuffer[0] ? LineBuffer : L"", + message ? L"\n\n" : L"", message ? message : traits::szEmptyString); if (IsDebuggerPresent()) { @@ -185,7 +187,7 @@ static int _CrtDbgReportWindow(int reportType, const char_t *filename, int linen tMessageBoxW messageBox = _CrtGetMessageBox(); if (!messageBox) - return IsDebuggerPresent() ? IDRETRY : IDABORT; + return (IsDebuggerPresent() ? IDRETRY : IDABORT); // TODO: If we are not interacive, add MB_SERVICE_NOTIFICATION return messageBox(NULL, szCompleteMessage, L"ReactOS C++ Runtime Library", @@ -206,7 +208,7 @@ static int _CrtEnterDbgReport(int reportType, const char_t *filename, int linenu { char LineBuffer[20] = {0}; - _itoa(linenumber, LineBuffer, 10); + _itoa_s(linenumber, LineBuffer, _countof(LineBuffer), 10); OutputDebugStringA("Nested Assert from File: "); traits::OutputDebugString(filename ? filename : traits::szUnknownFile); @@ -318,8 +320,8 @@ _VCrtDbgReportA( const char *format, va_list arglist) { - char szFormatted[DBGRPT_MAX_BUFFER_SIZE+1] = {0}; // The user provided message - char szCompleteMessage[(DBGRPT_MAX_BUFFER_SIZE+1)*2] = {0}; // The output for debug / file + char szFormatted[DBGRPT_MAX_BUFFER_SIZE] = {0}; // The user provided message + char szCompleteMessage[DBGRPT_MAX_BUFFER_SIZE] = {0}; // The output for debug / file // Check for recursive _CrtDbgReport calls, and validate reportType if (!_CrtEnterDbgReport(reportType, filename, linenumber)) @@ -327,31 +329,40 @@ _VCrtDbgReportA( if (filename) { - _snprintf(szCompleteMessage, DBGRPT_MAX_BUFFER_SIZE, "%s(%d) : ", filename, linenumber); + _snprintf_s(szCompleteMessage, + _countof(szCompleteMessage), + _countof(szCompleteMessage) - 1, + "%s(%d) : ", + filename, + linenumber); } if (format) { - int len = _vsnprintf(szFormatted, DBGRPT_MAX_BUFFER_SIZE - 2 - sizeof(DBGRPT_ASSERT_PREFIX_MESSAGE), format, arglist); + int len = _vsnprintf_s(szFormatted, + _countof(szFormatted), + _countof(szFormatted) - 2 - _countof(DBGRPT_ASSERT_PREFIX_MESSAGE), + format, + arglist); if (len < 0) { - strcpy(szFormatted, DBGRPT_STRING_TOO_LONG); + strcpy_s(szFormatted, _countof(szFormatted), DBGRPT_STRING_TOO_LONG); } if (reportType == _CRT_ASSERT) - strcat(szCompleteMessage, DBGRPT_ASSERT_PREFIX_MESSAGE); - strcat(szCompleteMessage, szFormatted); + strcat_s(szCompleteMessage, _countof(szCompleteMessage), DBGRPT_ASSERT_PREFIX_MESSAGE); + strcat_s(szCompleteMessage, _countof(szCompleteMessage), szFormatted); } else if (reportType == _CRT_ASSERT) { - strcat(szCompleteMessage, DBGRPT_ASSERT_PREFIX_NOMESSAGE); + strcat_s(szCompleteMessage, _countof(szCompleteMessage), DBGRPT_ASSERT_PREFIX_NOMESSAGE); } if (reportType == _CRT_ASSERT) { if (_CrtModeOutputFormat[reportType] & _CRTDBG_MODE_FILE) - strcat(szCompleteMessage, "\r"); - strcat(szCompleteMessage, "\n"); + strcat_s(szCompleteMessage, _countof(szCompleteMessage), "\r"); + strcat_s(szCompleteMessage, _countof(szCompleteMessage), "\n"); } // FIXME: Handle user report hooks here @@ -373,8 +384,8 @@ _VCrtDbgReportW( const wchar_t *format, va_list arglist) { - wchar_t szFormatted[DBGRPT_MAX_BUFFER_SIZE+1] = {0}; // The user provided message - wchar_t szCompleteMessage[(DBGRPT_MAX_BUFFER_SIZE+1)*2] = {0}; // The output for debug / file + wchar_t szFormatted[DBGRPT_MAX_BUFFER_SIZE] = {0}; // The user provided message + wchar_t szCompleteMessage[DBGRPT_MAX_BUFFER_SIZE] = {0}; // The output for debug / file // Check for recursive _CrtDbgReportW calls, and validate reportType if (!_CrtEnterDbgReport(reportType, filename, linenumber)) @@ -382,31 +393,40 @@ _VCrtDbgReportW( if (filename) { - _snwprintf(szCompleteMessage, DBGRPT_MAX_BUFFER_SIZE, L"%s(%d) : ", filename, linenumber); + _snwprintf_s(szCompleteMessage, + _countof(szCompleteMessage), + _countof(szCompleteMessage) - 1, + L"%s(%d) : ", + filename, + linenumber); } if (format) { - int len = _vsnwprintf(szFormatted, DBGRPT_MAX_BUFFER_SIZE - 2 - sizeof(DBGRPT_ASSERT_PREFIX_MESSAGE), format, arglist); + int len = _vsnwprintf_s(szFormatted, + _countof(szFormatted), + _countof(szFormatted) - 2 - _countof(DBGRPT_ASSERT_PREFIX_MESSAGE), + format, + arglist); if (len < 0) { - wcscpy(szFormatted, _CRT_WIDE(DBGRPT_STRING_TOO_LONG)); + wcscpy_s(szFormatted, _countof(szFormatted), _CRT_WIDE(DBGRPT_STRING_TOO_LONG)); } if (reportType == _CRT_ASSERT) - wcscat(szCompleteMessage, _CRT_WIDE(DBGRPT_ASSERT_PREFIX_MESSAGE)); - wcscat(szCompleteMessage, szFormatted); + wcscat_s(szCompleteMessage, _countof(szCompleteMessage), _CRT_WIDE(DBGRPT_ASSERT_PREFIX_MESSAGE)); + wcscat_s(szCompleteMessage, _countof(szCompleteMessage), szFormatted); } else if (reportType == _CRT_ASSERT) { - wcscat(szCompleteMessage, _CRT_WIDE(DBGRPT_ASSERT_PREFIX_NOMESSAGE)); + wcscat_s(szCompleteMessage, _countof(szCompleteMessage), _CRT_WIDE(DBGRPT_ASSERT_PREFIX_NOMESSAGE)); } if (reportType == _CRT_ASSERT) { if (_CrtModeOutputFormat[reportType] & _CRTDBG_MODE_FILE) - wcscat(szCompleteMessage, L"\r"); - wcscat(szCompleteMessage, L"\n"); + wcscat_s(szCompleteMessage, _countof(szCompleteMessage), L"\r"); + wcscat_s(szCompleteMessage, _countof(szCompleteMessage), L"\n"); } // FIXME: Handle user report hooks here