Re: [PATCH v2 2/4] msacm32: Add invalid parameter checks for acmFormatDetails().
This one adds test failures on my machine: msacm.c:542: Test failed: acmFormatDetailsA(): rc = 00000200, should be 0000000b msacm.c:542: Test failed: acmFormatDetailsA(): rc = 00000200, should be 0000000b msacm.c:542: Test failed: acmFormatDetailsA(): rc = 00000200, should be 0000000b msacm.c:542: Test failed: acmFormatDetailsA(): rc = 00000200, should be 0000000b msacm.c:542: Test failed: acmFormatDetailsA(): rc = 00000200, should be 0000000b msacm.c:542: Test failed: acmFormatDetailsA(): rc = 00000200, should be 0000000b Andrew On Wed, Jun 07, 2017 at 10:28:40AM -0500, Zebediah Figura wrote:
v2: restore zero struct, check for error from acmFormatTagDetailsW() Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/msacm32/format.c | 34 ++++++++++-- dlls/msacm32/tests/msacm.c | 132 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 163 insertions(+), 3 deletions(-)
diff --git a/dlls/msacm32/format.c b/dlls/msacm32/format.c index 902807cb72..359fbe8366 100644 --- a/dlls/msacm32/format.c +++ b/dlls/msacm32/format.c @@ -68,7 +68,7 @@ static BOOL CALLBACK MSACM_FillFormatTagsCB(HACMDRIVERID hadid, HACMDRIVER had;
if (acmDriverOpen(&had, hadid, 0) == MMSYSERR_NOERROR) { - ACMFORMATDETAILSW afd; + ACMFORMATDETAILSW afd = {0}; unsigned int i, len; MMRESULT mmr; WCHAR buffer[ACMFORMATDETAILS_FORMAT_CHARS+16]; @@ -87,6 +87,7 @@ static BOOL CALLBACK MSACM_FillFormatTagsCB(HACMDRIVERID hadid, int j;
afd.dwFormatIndex = i; + afd.fdwSupport = 0; mmr = acmFormatDetailsW(had, &afd, ACM_FORMATDETAILSF_INDEX); if (mmr == MMSYSERR_NOERROR) { lstrcpynW(buffer, afd.szFormat, ACMFORMATTAGDETAILS_FORMATTAG_CHARS + 1); @@ -112,7 +113,7 @@ static BOOL CALLBACK MSACM_FillFormatTagsCB(HACMDRIVERID hadid, HACMDRIVER had;
if (acmDriverOpen(&had, hadid, 0) == MMSYSERR_NOERROR) { - ACMFORMATDETAILSW afd; + ACMFORMATDETAILSW afd = {0};
afd.cbStruct = sizeof(afd); afd.dwFormatTag = paftd->dwFormatTag; @@ -376,10 +377,14 @@ MMRESULT WINAPI acmFormatDetailsA(HACMDRIVER had, PACMFORMATDETAILSA pafd, ACMFORMATDETAILSW afdw; MMRESULT mmr;
+ if (!pafd) + return MMSYSERR_INVALPARAM; + memset(&afdw, 0, sizeof(afdw)); afdw.cbStruct = sizeof(afdw); afdw.dwFormatIndex = pafd->dwFormatIndex; afdw.dwFormatTag = pafd->dwFormatTag; + afdw.fdwSupport = pafd->fdwSupport; afdw.pwfx = pafd->pwfx; afdw.cbwfx = pafd->cbwfx;
@@ -401,11 +406,24 @@ MMRESULT WINAPI acmFormatDetailsW(HACMDRIVER had, PACMFORMATDETAILSW pafd, DWORD MMRESULT mmr; static const WCHAR fmt1[] = {'%','d',' ','H','z',0}; static const WCHAR fmt2[] = {';',' ','%','d',' ','b','i','t','s',0}; + ACMFORMATTAGDETAILSW aftd = {0};
TRACE("(%p, %p, %d)\n", had, pafd, fdwDetails);
+ if (!pafd) + return MMSYSERR_INVALPARAM; + if (pafd->cbStruct < sizeof(*pafd)) return MMSYSERR_INVALPARAM;
+ if (!pafd->pwfx) + return MMSYSERR_INVALPARAM; + + if (pafd->cbwfx < sizeof(PCMWAVEFORMAT)) + return MMSYSERR_INVALPARAM; + + if (pafd->fdwSupport) + return MMSYSERR_INVALPARAM; + switch (fdwDetails) { case ACM_FORMATDETAILSF_FORMAT: if (pafd->dwFormatTag != pafd->pwfx->wFormatTag) { @@ -430,7 +448,16 @@ MMRESULT WINAPI acmFormatDetailsW(HACMDRIVER had, PACMFORMATDETAILSW pafd, DWORD } break; case ACM_FORMATDETAILSF_INDEX: - /* should check pafd->dwFormatIndex < aftd->cStandardFormats */ + aftd.cbStruct = sizeof(aftd); + aftd.dwFormatTag = pafd->dwFormatTag; + mmr = acmFormatTagDetailsW(had, &aftd, ACM_FORMATTAGDETAILSF_FORMATTAG); + if (mmr != MMSYSERR_NOERROR) + break; + if (pafd->dwFormatIndex >= aftd.cStandardFormats) + { + mmr = MMSYSERR_INVALPARAM; + break; + } mmr = MSACM_Message(had, ACMDM_FORMAT_DETAILS, (LPARAM)pafd, fdwDetails); break; default: @@ -581,6 +608,7 @@ static BOOL MSACM_FormatEnumHelper(PWINE_ACMDRIVERID padid, HACMDRIVER had, for (j = 0; j < aftd.cStandardFormats; j++) { pafd->dwFormatIndex = j; pafd->dwFormatTag = aftd.dwFormatTag; + pafd->fdwSupport = 0; if (acmFormatDetailsW(had, pafd, ACM_FORMATDETAILSF_INDEX) != MMSYSERR_NOERROR) continue;
diff --git a/dlls/msacm32/tests/msacm.c b/dlls/msacm32/tests/msacm.c index f5ab168290..1b47fe2087 100644 --- a/dlls/msacm32/tests/msacm.c +++ b/dlls/msacm32/tests/msacm.c @@ -38,9 +38,110 @@ static BOOL CALLBACK FormatTagEnumProc(HACMDRIVERID hadid, DWORD_PTR dwInstance, DWORD fdwSupport) { + MMRESULT rc; + HACMDRIVER had; + if (winetest_interactive) trace(" Format 0x%04x: %s\n", paftd->dwFormatTag, paftd->szFormatTag);
+ rc = acmDriverOpen(&had, hadid, 0); + ok(rc == MMSYSERR_NOERROR || rc == MMSYSERR_NODRIVER, + "acmDriverOpen(): rc = %08x, should be %08x\n", + rc, MMSYSERR_NOERROR); + + if (rc == MMSYSERR_NOERROR) + { + ACMFORMATDETAILSA fd = {0}; + WAVEFORMATEX *pwfx; + DWORD i; + + fd.cbStruct = sizeof(fd); + if (paftd->cbFormatSize < sizeof(WAVEFORMATEX)) + pwfx = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(WAVEFORMATEX)); + else + pwfx = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, paftd->cbFormatSize); + fd.pwfx = pwfx; + fd.cbwfx = paftd->cbFormatSize; + fd.dwFormatTag = paftd->dwFormatTag; + + /* try bad pwfx */ + fd.pwfx = NULL; + rc = acmFormatDetailsA(had, &fd, ACM_FORMATDETAILSF_FORMAT); + ok(rc == MMSYSERR_INVALPARAM, + "acmFormatDetailsA(): rc = %08x, should be %08x\n", + rc, MMSYSERR_INVALPARAM); + fd.pwfx = pwfx; + + /* try bad wFormatTag */ + fd.pwfx->wFormatTag = WAVE_FORMAT_UNKNOWN; + rc = acmFormatDetailsA(had, &fd, ACM_FORMATDETAILSF_FORMAT); + ok(rc == MMSYSERR_INVALPARAM, + "acmFormatDetailsA(): rc = %08x, should be %08x\n", + rc, MMSYSERR_INVALPARAM); + fd.pwfx->wFormatTag = paftd->dwFormatTag; + + /* try bad fdwSupport */ + fd.fdwSupport = 0xdeadbeef; + rc = acmFormatDetailsA(had, &fd, ACM_FORMATDETAILSF_FORMAT); + ok(rc == MMSYSERR_INVALPARAM, + "acmFormatDetailsA(): rc = %08x, should be %08x\n", + rc, MMSYSERR_INVALPARAM); + fd.fdwSupport = 0; + + /* try bad pwfx structure size */ + fd.cbwfx = sizeof(PCMWAVEFORMAT)-1; + rc = acmFormatDetailsA(had, &fd, ACM_FORMATDETAILSF_FORMAT); + ok(rc == MMSYSERR_INVALPARAM, + "acmFormatDetailsA(): rc = %08x, should be %08x\n", + rc, MMSYSERR_INVALPARAM); + fd.cbwfx = paftd->cbFormatSize; + + /* test bad parameters (all zero) */ + rc = acmFormatDetailsA(had, &fd, ACM_FORMATDETAILSF_FORMAT); + ok(rc == ACMERR_NOTPOSSIBLE, + "acmFormatDetailsA(): rc = %08x, should be %08x\n", + rc, ACMERR_NOTPOSSIBLE); + + /* test index */ + for (i = 0; i < paftd->cStandardFormats; i++) + { + fd.dwFormatIndex = i; + + fd.fdwSupport = 0; + fd.cbwfx = paftd->cbFormatSize; + fd.pwfx->cbSize = 0xbeef; + rc = acmFormatDetailsA(had, &fd, ACM_FORMATDETAILSF_INDEX); + ok(rc == MMSYSERR_NOERROR, + "acmFormatDetailsA(): rc = %08x, should be %08x\n", + rc, MMSYSERR_NOERROR); + + /* Windows will write cbSize (and other data) even if the + * given cbwfx is not large enough */ + fd.fdwSupport = 0; + fd.cbwfx = sizeof(PCMWAVEFORMAT); + fd.pwfx->cbSize = 0xbeef; + rc = acmFormatDetailsA(had, &fd, ACM_FORMATDETAILSF_INDEX); + todo_wine_if(rc != MMSYSERR_NOERROR) /* remove when fixed */ + ok(rc == MMSYSERR_NOERROR, + "acmFormatDetailsA(): rc = %08x, should be %08x\n", + rc, MMSYSERR_NOERROR); + if (paftd->dwFormatTag != WAVE_FORMAT_PCM) + todo_wine_if(fd.pwfx->cbSize != paftd->cbFormatSize - sizeof(WAVEFORMATEX)) /* remove when fixed */ + ok(fd.pwfx->cbSize == paftd->cbFormatSize - sizeof(WAVEFORMATEX), + "expected cbSize %d, got %d\n", + paftd->cbFormatSize - sizeof(WAVEFORMATEX), fd.pwfx->cbSize); + } + + /* one more */ + fd.dwFormatIndex = paftd->cStandardFormats; + fd.fdwSupport = 0; + rc = acmFormatDetailsA(had, &fd, ACM_FORMATDETAILSF_INDEX); + ok(rc == MMSYSERR_INVALPARAM, + "acmFormatDetailsA(): rc = %08x, should be %08x\n", + rc, MMSYSERR_INVALPARAM); + + HeapFree(GetProcessHeap(), 0, pwfx); + } return TRUE; }
@@ -411,6 +512,37 @@ static BOOL CALLBACK DriverEnumProc(HACMDRIVERID hadid, "acmFormatTagEnumA(): rc = %08x, should be %08x\n", rc, MMSYSERR_NOERROR);
+ /* try bad pointer */ + rc = acmFormatDetailsA(had, NULL, ACM_FORMATDETAILSF_INDEX); + ok(rc == MMSYSERR_INVALPARAM, + "acmFormatDetailsA(): rc = %08x, should be %08x\n", + rc, MMSYSERR_INVALPARAM); + + /* try bad structure size */ + ZeroMemory(&fd, sizeof(fd)); + rc = acmFormatDetailsA(had, &fd, ACM_FORMATDETAILSF_INDEX); + ok(rc == MMSYSERR_INVALPARAM, + "acmFormatDetailsA(): rc = %08x, should be %08x\n", + rc, MMSYSERR_INVALPARAM); + + fd.cbStruct = sizeof(fd) - 1; + rc = acmFormatDetailsA(had, &fd, ACM_FORMATDETAILSF_INDEX); + ok(rc == MMSYSERR_INVALPARAM, + "acmFormatDetailsA(): rc = %08x, should be %08x\n", + rc, MMSYSERR_INVALPARAM); + + fd.cbStruct = sizeof(fd); + fd.pwfx = pwfx; + ZeroMemory(fd.pwfx, dwSize); + fd.cbwfx = dwSize; + fd.dwFormatTag = WAVE_FORMAT_UNKNOWN; + + /* try WAVE_FORMAT_UNKNOWN */ + rc = acmFormatDetailsA(had, &fd, ACM_FORMATDETAILSF_INDEX); + ok(rc == MMSYSERR_INVALPARAM, + "acmFormatDetailsA(): rc = %08x, should be %08x\n", + rc, MMSYSERR_INVALPARAM); + HeapFree(GetProcessHeap(), 0, pwfx);
/* try invalid handle */ -- 2.13.0
participants (1)
-
Andrew Eikum