mirror of https://github.com/FreeRDP/FreeRDP
winpr/path: Denounce PathAllocCombine and more
Note: This commit does NOT really fix PathAllocCombine! - print a warning message that the function is buggy and added a code comment describing the issues - fix misuse of the S_FALSE HRESULT in error conditions - prevent some segfaults - check result of HeapAlloc Fortunately PathAllocCombine is unused in FreeRDP
This commit is contained in:
Norbert Federa
parent
6823cb4a86
commit
cdabfcf9e1
|
|
@ -6,6 +6,18 @@
|
|||
#define PATH_ALLOC_COMBINE PathAllocCombineA
|
||||
*/
|
||||
|
||||
/**
|
||||
* FIXME: These implementations of the PathAllocCombine functions have
|
||||
* several issues:
|
||||
* - pszPathIn or pszMore may be NULL (but not both)
|
||||
* - no check if pszMore is fully qualified (if so, it must be directly
|
||||
* copied to the output buffer without being combined with pszPathIn.
|
||||
* - if pszMore begins with a _single_ backslash it must be combined with
|
||||
* only the root of the path pointed to by pszPathIn and there's no code
|
||||
* to extract the root of pszPathIn.
|
||||
* - the function will crash with some short string lengths of the parameters
|
||||
*/
|
||||
|
||||
#if DEFINE_UNICODE
|
||||
|
||||
HRESULT PATH_ALLOC_COMBINE(PCWSTR pszPathIn, PCWSTR pszMore, unsigned long dwFlags, PWSTR* ppszPathOut)
|
||||
|
|
@ -18,14 +30,26 @@ HRESULT PATH_ALLOC_COMBINE(PCWSTR pszPathIn, PCWSTR pszMore, unsigned long dwFla
|
|||
int pszPathInLength;
|
||||
int pszPathOutLength;
|
||||
|
||||
if (!pszPathIn)
|
||||
return S_FALSE;
|
||||
WLog_WARN(TAG, "%s: has known bugs and needs fixing.", __FUNCTION__);
|
||||
|
||||
if (!ppszPathOut)
|
||||
return E_INVALIDARG;
|
||||
|
||||
if (!pszPathIn && !pszMore)
|
||||
return E_INVALIDARG;
|
||||
|
||||
if (!pszMore)
|
||||
return S_FALSE;
|
||||
return E_FAIL; /* valid but not implemented, see top comment */
|
||||
|
||||
pszPathInLength = lstrlenW(pszPathIn);
|
||||
pszMoreLength = lstrlenW(pszMore);
|
||||
if (!pszPathIn)
|
||||
return E_FAIL; /* valid but not implemented, see top comment */
|
||||
|
||||
pszPathInLength = lstrlenA(pszPathIn);
|
||||
pszMoreLength = lstrlenA(pszMore);
|
||||
|
||||
/* prevent segfaults - the complete implementation below is buggy */
|
||||
if (pszPathInLength < 3)
|
||||
return E_FAIL;
|
||||
|
||||
backslashIn = (pszPathIn[pszPathInLength - 1] == _PATH_SEPARATOR_CHR) ? TRUE : FALSE;
|
||||
backslashMore = (pszMore[0] == _PATH_SEPARATOR_CHR) ? TRUE : FALSE;
|
||||
|
|
@ -40,6 +64,9 @@ HRESULT PATH_ALLOC_COMBINE(PCWSTR pszPathIn, PCWSTR pszMore, unsigned long dwFla
|
|||
sizeOfBuffer = (pszPathOutLength + 1) * 2;
|
||||
|
||||
pszPathOut = (PWSTR) HeapAlloc(GetProcessHeap(), 0, sizeOfBuffer * 2);
|
||||
if (!pszPathOut)
|
||||
return E_OUTOFMEMORY;
|
||||
|
||||
swprintf_s(pszPathOut, sizeOfBuffer, L"%c:%s", pszPathIn[0], pszMore);
|
||||
|
||||
*ppszPathOut = pszPathOut;
|
||||
|
|
@ -55,6 +82,8 @@ HRESULT PATH_ALLOC_COMBINE(PCWSTR pszPathIn, PCWSTR pszMore, unsigned long dwFla
|
|||
sizeOfBuffer = (pszPathOutLength + 1) * 2;
|
||||
|
||||
pszPathOut = (PWSTR) HeapAlloc(GetProcessHeap(), 0, sizeOfBuffer * 2);
|
||||
if (!pszPathOut)
|
||||
return E_OUTOFMEMORY;
|
||||
|
||||
if (backslashIn)
|
||||
swprintf_s(pszPathOut, sizeOfBuffer, L"%s%s", pszPathIn, pszMore);
|
||||
|
|
@ -67,7 +96,7 @@ HRESULT PATH_ALLOC_COMBINE(PCWSTR pszPathIn, PCWSTR pszMore, unsigned long dwFla
|
|||
}
|
||||
#endif
|
||||
|
||||
return S_OK;
|
||||
return E_FAIL;
|
||||
}
|
||||
|
||||
#else
|
||||
|
|
@ -81,15 +110,27 @@ HRESULT PATH_ALLOC_COMBINE(PCSTR pszPathIn, PCSTR pszMore, unsigned long dwFlags
|
|||
int pszPathInLength;
|
||||
int pszPathOutLength;
|
||||
|
||||
if (!pszPathIn)
|
||||
return S_FALSE;
|
||||
WLog_WARN(TAG, "%s: has known bugs and needs fixing.", __FUNCTION__);
|
||||
|
||||
if (!ppszPathOut)
|
||||
return E_INVALIDARG;
|
||||
|
||||
if (!pszPathIn && !pszMore)
|
||||
return E_INVALIDARG;
|
||||
|
||||
if (!pszMore)
|
||||
return S_FALSE;
|
||||
return E_FAIL; /* valid but not implemented, see top comment */
|
||||
|
||||
if (!pszPathIn)
|
||||
return E_FAIL; /* valid but not implemented, see top comment */
|
||||
|
||||
pszPathInLength = lstrlenA(pszPathIn);
|
||||
pszMoreLength = lstrlenA(pszMore);
|
||||
|
||||
/* prevent segfaults - the complete implementation below is buggy */
|
||||
if (pszPathInLength < 3)
|
||||
return E_FAIL;
|
||||
|
||||
backslashIn = (pszPathIn[pszPathInLength - 1] == _PATH_SEPARATOR_CHR) ? TRUE : FALSE;
|
||||
backslashMore = (pszMore[0] == _PATH_SEPARATOR_CHR) ? TRUE : FALSE;
|
||||
|
||||
|
|
@ -103,6 +144,9 @@ HRESULT PATH_ALLOC_COMBINE(PCSTR pszPathIn, PCSTR pszMore, unsigned long dwFlags
|
|||
sizeOfBuffer = (pszPathOutLength + 1) * 2;
|
||||
|
||||
pszPathOut = (PSTR) HeapAlloc(GetProcessHeap(), 0, sizeOfBuffer * 2);
|
||||
if (!pszPathOut)
|
||||
return E_OUTOFMEMORY;
|
||||
|
||||
sprintf_s(pszPathOut, sizeOfBuffer, "%c:%s", pszPathIn[0], pszMore);
|
||||
|
||||
*ppszPathOut = pszPathOut;
|
||||
|
|
@ -118,6 +162,8 @@ HRESULT PATH_ALLOC_COMBINE(PCSTR pszPathIn, PCSTR pszMore, unsigned long dwFlags
|
|||
sizeOfBuffer = (pszPathOutLength + 1) * 2;
|
||||
|
||||
pszPathOut = (PSTR) HeapAlloc(GetProcessHeap(), 0, sizeOfBuffer * 2);
|
||||
if (!pszPathOut)
|
||||
return E_OUTOFMEMORY;
|
||||
|
||||
if (backslashIn)
|
||||
sprintf_s(pszPathOut, sizeOfBuffer, "%s%s", pszPathIn, pszMore);
|
||||
|
|
@ -129,7 +175,7 @@ HRESULT PATH_ALLOC_COMBINE(PCSTR pszPathIn, PCSTR pszMore, unsigned long dwFlags
|
|||
return S_OK;
|
||||
}
|
||||
|
||||
return S_OK;
|
||||
return E_FAIL;
|
||||
}
|
||||
|
||||
#endif
|
||||
|
|
|
|||
Loading…
Reference in New Issue