mir/libreoffice
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Make Windows error reporting more robust

Browse Source 
https://msdn.microsoft.com/en-us/library/ms679351 describes that
"it is unsafe to take an arbitrary system error code returned from an API
and use FORMAT_MESSAGE_FROM_SYSTEM without FORMAT_MESSAGE_IGNORE_INSERTS"
Previously in case when an error string would contain inserts, function
returned error, so the error message wasn't shown (at least it didn't
crash, thanks to nullptr as the function's last argument).

As the function may fail, we now pre-nullify the buffer pointer to avoid
dereferencing uninitialized pointer later (though at least for some
Windows versions, the function nullifies the pointer in case of
FORMAT_MESSAGE_ALLOCATE_BUFFER, but there's no explicit guarantee of this).

Also release of allocated buffer is changed to recommended use of
HeapFree.

The code that doesn't make use of OUString is left directly calling
FormatMessage, to avoid introducing new dependencies. Where it makes
sense, we now use WindowsErrorString from <comphelper/windowserrorstring.hxx>

Change-Id: I834c08eb6d92987e7d3d01e2c36ec55e42aea848
Reviewed-on: https://gerrit.libreoffice.org/44206
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
This commit is contained in:
Mike Kaganski
2017-11-02 13:22:41 +03:00
parent fce7a0b360
commit e3530d2c9d
10 changed files with 24 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
comphelper/source/windows/windows_process.cxx
View File

@@ -254,7 +254,7 @@ WinLaunchChild(const wchar_t *exePath,
nullptr); nullptr);
wprintf(L"Error restarting: %s\n", lpMsgBuf ? lpMsgBuf : L"(null)"); wprintf(L"Error restarting: %s\n", lpMsgBuf ? lpMsgBuf : L"(null)");
if (lpMsgBuf) if (lpMsgBuf)
LocalFree(lpMsgBuf); HeapFree(GetProcessHeap(), 0, lpMsgBuf);
} }
free(cl); free(cl);

7
dbaccess/win32/source/odbcconfig/odbcconfig.cxx
View File

@@ -44,10 +44,9 @@ int displayLastError()
{ {
DWORD dwError = GetLastError(); DWORD dwError = GetLastError();
LPVOID lpMsgBuf; LPVOID lpMsgBuf = nullptr;
FormatMessageW( FormatMessageW(
FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
FORMAT_MESSAGE_FROM_SYSTEM,
nullptr, nullptr,
dwError, dwError,
MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), // Default language MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), // Default language
@@ -60,7 +59,7 @@ int displayLastError()
MessageBoxW( nullptr, static_cast<LPCWSTR>(lpMsgBuf), nullptr, MB_OK | MB_ICONERROR ); MessageBoxW( nullptr, static_cast<LPCWSTR>(lpMsgBuf), nullptr, MB_OK | MB_ICONERROR );
// Free the buffer. // Free the buffer.
LocalFree( lpMsgBuf ); HeapFree( GetProcessHeap(), 0, lpMsgBuf );
return dwError; return dwError;
} }

7
desktop/win32/source/applauncher/launcher.cxx
View File

@@ -81,11 +81,10 @@ extern "C" int APIENTRY wWinMain( HINSTANCE, HINSTANCE, LPWSTR, int )
DWORD dwError = GetLastError(); DWORD dwError = GetLastError();
LPWSTR lpMsgBuf; LPWSTR lpMsgBuf = nullptr;
FormatMessageW( FormatMessageW(
FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
FORMAT_MESSAGE_FROM_SYSTEM,
nullptr, nullptr,
dwError, dwError,
MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), // Default language MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), // Default language
@@ -98,7 +97,7 @@ extern "C" int APIENTRY wWinMain( HINSTANCE, HINSTANCE, LPWSTR, int )
MessageBoxW( nullptr, lpMsgBuf, nullptr, MB_OK | MB_ICONERROR ); MessageBoxW( nullptr, lpMsgBuf, nullptr, MB_OK | MB_ICONERROR );
// Free the buffer. // Free the buffer.
LocalFree( lpMsgBuf ); HeapFree( GetProcessHeap(), 0, lpMsgBuf );
return dwError; return dwError;
} }

4
desktop/win32/source/loader.cxx
View File

@@ -27,10 +27,10 @@ void fail()
{ {
LPWSTR buf = nullptr; LPWSTR buf = nullptr;
FormatMessageW( FormatMessageW(
FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM, nullptr, FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, nullptr,
GetLastError(), 0, reinterpret_cast< LPWSTR >(&buf), 0, nullptr); GetLastError(), 0, reinterpret_cast< LPWSTR >(&buf), 0, nullptr);
MessageBoxW(nullptr, buf, nullptr, MB_OK | MB_ICONERROR); MessageBoxW(nullptr, buf, nullptr, MB_OK | MB_ICONERROR);
LocalFree(buf); HeapFree(GetProcessHeap(), 0, buf);
TerminateProcess(GetCurrentProcess(), 255); TerminateProcess(GetCurrentProcess(), 255);
} }

7
extensions/source/activex/SOActiveX.cxx
View File

@@ -44,10 +44,9 @@
void OutputError_Impl( HWND hw, HRESULT ErrorCode ) void OutputError_Impl( HWND hw, HRESULT ErrorCode )
{ {
LPWSTR sMessage; LPWSTR sMessage = nullptr;
FormatMessageW( FormatMessageW(
FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
FORMAT_MESSAGE_FROM_SYSTEM,
nullptr, nullptr,
ErrorCode, ErrorCode,
MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), // Default language MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), // Default language
@@ -56,7 +55,7 @@ void OutputError_Impl( HWND hw, HRESULT ErrorCode )
nullptr nullptr
); );
MessageBoxW( hw, sMessage, nullptr, MB_OK | MB_ICONINFORMATION ); MessageBoxW( hw, sMessage, nullptr, MB_OK | MB_ICONINFORMATION );
LocalFree( sMessage ); HeapFree( GetProcessHeap(), 0, sMessage );
} }
HRESULT ExecuteFunc( IDispatch* idispUnoObject, HRESULT ExecuteFunc( IDispatch* idispUnoObject,

3
include/LibreOfficeKit/LibreOfficeKitInit.h
View File

@@ -100,7 +100,8 @@ extern "C"
static char *lok_dlerror(void) static char *lok_dlerror(void)
{ {
LPSTR buf = NULL; LPSTR buf = NULL;
FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM, NULL, GetLastError(), 0, reinterpret_cast<LPSTR>(&buf), 0, NULL); FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
NULL, GetLastError(), 0, reinterpret_cast<LPSTR>(&buf), 0, NULL);
return buf; return buf;
} }

4
include/comphelper/windowserrorstring.hxx
View File

@@ -21,7 +21,7 @@ inline OUString WindowsErrorString(DWORD nErrorCode)
{ {
LPWSTR pMsgBuf; LPWSTR pMsgBuf;
if (FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM, if (FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
nullptr, nullptr,
nErrorCode, nErrorCode,
MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
@@ -33,7 +33,7 @@ inline OUString WindowsErrorString(DWORD nErrorCode)
OUString result(o3tl::toU(pMsgBuf)); OUString result(o3tl::toU(pMsgBuf));
result.endsWith("\r\n", &result); result.endsWith("\r\n", &result);
LocalFree(pMsgBuf); HeapFree(GetProcessHeap(), 0, pMsgBuf);
return result; return result;
} }

9
sal/osl/w32/socket.cxx
View File

@@ -24,6 +24,7 @@
#include <rtl/alloc.h> #include <rtl/alloc.h>
#include <sal/log.hxx> #include <sal/log.hxx>
#include <o3tl/char16_t2wchar_t.hxx> #include <o3tl/char16_t2wchar_t.hxx>
#include <comphelper/windowserrorstring.hxx>
#include "sockimpl.hxx" #include "sockimpl.hxx"
@@ -791,13 +792,7 @@ oslSocket SAL_CALL osl_createSocket(
if(pSocket->m_Socket == OSL_INVALID_SOCKET) if(pSocket->m_Socket == OSL_INVALID_SOCKET)
{ {
int nErrno = WSAGetLastError(); int nErrno = WSAGetLastError();
wchar_t *sErr = nullptr; SAL_WARN("sal.osl", "socket creation failed: (" << nErrno << "): " << WindowsErrorString(nErrno));
FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
nullptr, nErrno,
MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
reinterpret_cast<LPWSTR>(&sErr), 0, nullptr);
SAL_WARN("sal.osl", "socket creation failed: (" << nErrno << ") " << sErr);
LocalFree(sErr);
destroySocketImpl(pSocket); destroySocketImpl(pSocket);
pSocket = nullptr; pSocket = nullptr;

4
shell/source/win32/spsupp/COMOpenDocuments.cxx
View File

@@ -51,7 +51,7 @@ HRESULT LOStart(const wchar_t* sModeArg, const wchar_t* sFilePath, bool bDoSecur
if (CreateProcessW(nullptr, sCommandLine, nullptr, nullptr, FALSE, 0, nullptr, nullptr, &si, &pi) == FALSE) if (CreateProcessW(nullptr, sCommandLine, nullptr, nullptr, FALSE, 0, nullptr, nullptr, &si, &pi) == FALSE)
{ {
DWORD dwError = GetLastError(); DWORD dwError = GetLastError();
wchar_t* sMsgBuf; wchar_t* sMsgBuf = nullptr;
FormatMessageW( FormatMessageW(
FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_ALLOCATE_BUFFER |
FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_FROM_SYSTEM |
@@ -65,7 +65,7 @@ HRESULT LOStart(const wchar_t* sModeArg, const wchar_t* sFilePath, bool bDoSecur
size_t nBufSize = wcslen(sMsgBuf) + 100; size_t nBufSize = wcslen(sMsgBuf) + 100;
std::vector<wchar_t> sDisplayBuf(nBufSize); std::vector<wchar_t> sDisplayBuf(nBufSize);
swprintf(sDisplayBuf.data(), nBufSize, L"Could not start LibreOffice. Error is 0x%08X:\n\n%s", dwError, sMsgBuf); swprintf(sDisplayBuf.data(), nBufSize, L"Could not start LibreOffice. Error is 0x%08X:\n\n%s", dwError, sMsgBuf);
LocalFree(sMsgBuf); HeapFree(GetProcessHeap(), 0, sMsgBuf);
// Report the error to user and return error // Report the error to user and return error
MessageBoxW(nullptr, sDisplayBuf.data(), nullptr, MB_ICONERROR); MessageBoxW(nullptr, sDisplayBuf.data(), nullptr, MB_ICONERROR);

14
vcl/win/window/salobj.cxx
View File

@@ -29,6 +29,8 @@
#include <win/salframe.h> #include <win/salframe.h>
#include <win/salobj.h> #include <win/salobj.h>
#include <comphelper/windowserrorstring.hxx>
static bool ImplIsSysWindowOrChild( HWND hWndParent, HWND hWndChild ) static bool ImplIsSysWindowOrChild( HWND hWndParent, HWND hWndChild )
{ {
if ( hWndParent == hWndChild ) if ( hWndParent == hWndChild )
@@ -522,16 +524,8 @@ SalObject* ImplSalCreateObject( WinSalInstance* pInst, WinSalFrame* pParent )
if ( !hWndChild ) if ( !hWndChild )
{ {
#if OSL_DEBUG_LEVEL > 1 SAL_WARN("vcl", "CreateWindowExW failed: " << WindowsErrorString(GetLastError()));
wchar_t *msg = NULL;
FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER
|FORMAT_MESSAGE_IGNORE_INSERTS
|FORMAT_MESSAGE_FROM_SYSTEM,
NULL, GetLastError(), 0,
(LPWSTR) &msg, 0, NULL);
MessageBoxW(NULL, msg, L"CreateWindowExW failed", MB_OK);
HeapFree(GetProcessHeap(), msg);
#endif
delete pObject; delete pObject;
return nullptr; return nullptr;
} }