From 78b0fb4a21bab47d05b07956115fbe4e5491dfa8 Mon Sep 17 00:00:00 2001 From: Johannes Anderwald Date: Sat, 12 Jun 2010 10:21:03 +0000 Subject: [PATCH] [WDMAUD_KERNEL] - Fix possible buffer overflow [MMIXER] - Add sanity checks svn path=/trunk/; revision=47763 --- .../wdm/audio/legacy/wdmaud/deviface.c | 26 ++++++++++++++----- reactos/lib/drivers/sound/mmixer/controls.c | 17 +++++++++--- reactos/lib/drivers/sound/mmixer/filter.c | 3 +++ reactos/lib/drivers/sound/mmixer/mixer.c | 2 ++ reactos/lib/drivers/sound/mmixer/priv.h | 1 + reactos/lib/drivers/sound/mmixer/sup.c | 4 ++- reactos/lib/drivers/sound/mmixer/wave.c | 5 ++-- 7 files changed, 44 insertions(+), 14 deletions(-) diff --git a/reactos/drivers/wdm/audio/legacy/wdmaud/deviface.c b/reactos/drivers/wdm/audio/legacy/wdmaud/deviface.c index d191a97c2db..ca4f8834765 100644 --- a/reactos/drivers/wdm/audio/legacy/wdmaud/deviface.c +++ b/reactos/drivers/wdm/audio/legacy/wdmaud/deviface.c @@ -122,18 +122,32 @@ WdmAudOpenSysAudioDevices( } else { - Length = wcslen(DeviceName.Buffer) + 1; - Entry = (SYSAUDIO_ENTRY*)AllocateItem(NonPagedPool, sizeof(SYSAUDIO_ENTRY) + Length * sizeof(WCHAR)); + Entry = (SYSAUDIO_ENTRY*)AllocateItem(NonPagedPool, sizeof(SYSAUDIO_ENTRY)); if (!Entry) { return STATUS_INSUFFICIENT_RESOURCES; } - Entry->SymbolicLink.Length = Entry->SymbolicLink.MaximumLength = Length * sizeof(WCHAR); - Entry->SymbolicLink.MaximumLength += sizeof(WCHAR); - Entry->SymbolicLink.Buffer = (LPWSTR) (Entry + 1); - wcscpy(Entry->SymbolicLink.Buffer, DeviceName.Buffer); + Length = wcslen(DeviceName.Buffer) + 1; + Entry->SymbolicLink.Length = 0; + Entry->SymbolicLink.MaximumLength = Length * sizeof(WCHAR); + Entry->SymbolicLink.Buffer = AllocateItem(NonPagedPool, Entry->SymbolicLink.MaximumLength); + + if (!Entry->SymbolicLink.Buffer) + { + FreeItem(Entry); + return STATUS_INSUFFICIENT_RESOURCES; + } + + Status = RtlAppendUnicodeStringToString(&Entry->SymbolicLink, &DeviceName); + + if (!NT_SUCCESS(Status)) + { + FreeItem(Entry->SymbolicLink.Buffer); + FreeItem(Entry); + return Status; + } InsertTailList(&DeviceExtension->SysAudioDeviceList, &Entry->Entry); DeviceExtension->NumSysAudioDevices++; diff --git a/reactos/lib/drivers/sound/mmixer/controls.c b/reactos/lib/drivers/sound/mmixer/controls.c index 36794dfc69a..5b0ac71ed34 100644 --- a/reactos/lib/drivers/sound/mmixer/controls.c +++ b/reactos/lib/drivers/sound/mmixer/controls.c @@ -15,6 +15,7 @@ MMixerGetTargetPinsByNodeConnectionIndex( IN PKSMULTIPLE_ITEM NodeTypes, IN ULONG bUpDirection, IN ULONG NodeConnectionIndex, + IN ULONG PinCount, OUT PULONG Pins) { PKSTOPOLOGY_CONNECTION Connection; @@ -41,6 +42,9 @@ MMixerGetTargetPinsByNodeConnectionIndex( //DPRINT("GetTargetPinsByNodeIndex FOUND Target Pin %u Parsed %u\n", PinId, Pins[PinId]); + // sanity check + ASSERT(PinId < PinCount); + /* mark pin index as a target pin */ Pins[PinId] = TRUE; return MM_STATUS_SUCCESS; @@ -61,7 +65,7 @@ MMixerGetTargetPinsByNodeConnectionIndex( for(Index = 0; Index < NodeConnectionCount; Index++) { // iterate recursively into the nodes - Status = MMixerGetTargetPinsByNodeConnectionIndex(MixerContext, NodeConnections, NodeTypes, bUpDirection, NodeConnection[Index], Pins); + Status = MMixerGetTargetPinsByNodeConnectionIndex(MixerContext, NodeConnections, NodeTypes, bUpDirection, NodeConnection[Index], PinCount, Pins); ASSERT(Status == MM_STATUS_SUCCESS); } // free node connection indexes @@ -597,6 +601,8 @@ MMixerCreateDestinationLine( DestinationLine->Line.Target.wMid = MixerInfo->MixCaps.wMid; DestinationLine->Line.Target.wPid = MixerInfo->MixCaps.wPid; DestinationLine->Line.Target.vDriverVersion = MixerInfo->MixCaps.vDriverVersion; + + ASSERT(MixerInfo->MixCaps.szPname[MAXPNAMELEN-1] == 0); wcscpy(DestinationLine->Line.Target.szPname, MixerInfo->MixCaps.szPname); // initialize extra line @@ -736,11 +742,11 @@ MMixerHandlePhysicalConnection( return Status; } - /* there should be no split in the bride pin */ + /* there should be no split in the bridge pin */ ASSERT(PinConnectionIndexCount == 1); /* find all target pins of this connection */ - Status = MMixerGetTargetPinsByNodeConnectionIndex(MixerContext, NodeConnections, NodeTypes, FALSE, PinConnectionIndex[0], PinsRef); + Status = MMixerGetTargetPinsByNodeConnectionIndex(MixerContext, NodeConnections, NodeTypes, FALSE, PinConnectionIndex[0], PinsRefCount, PinsRef); if (Status != MM_STATUS_SUCCESS) { MixerContext->Free(PinsRef); @@ -779,7 +785,7 @@ MMixerHandlePhysicalConnection( } // now get all connected source pins - Status = MMixerGetTargetPinsByNodeConnectionIndex(MixerContext, NodeConnections, NodeTypes, TRUE, MixerControls[0], PinsSrcRef); + Status = MMixerGetTargetPinsByNodeConnectionIndex(MixerContext, NodeConnections, NodeTypes, TRUE, MixerControls[0], PinsRefCount, PinsSrcRef); if (Status != MM_STATUS_SUCCESS) { // failed */ @@ -857,6 +863,9 @@ MMixerInitializeFilter( InitializeListHead(&MixerInfo->LineList); InitializeListHead(&MixerInfo->EventList); + // sanity check + ASSERT(PinCount); + // now allocate an array which will receive the indices of the pin // which has a ADC / DAC nodetype in its path Pins = (PULONG)MixerContext->Alloc(PinCount * sizeof(ULONG)); diff --git a/reactos/lib/drivers/sound/mmixer/filter.c b/reactos/lib/drivers/sound/mmixer/filter.c index d02477de082..955fd273ae9 100644 --- a/reactos/lib/drivers/sound/mmixer/filter.c +++ b/reactos/lib/drivers/sound/mmixer/filter.c @@ -57,6 +57,9 @@ MMixerGetFilterTopologyProperty( if (Status != MM_STATUS_MORE_ENTRIES) return Status; + //sanity check + ASSERT(BytesReturned); + // allocate an result buffer MultipleItem = (PKSMULTIPLE_ITEM)MixerContext->Alloc(BytesReturned); diff --git a/reactos/lib/drivers/sound/mmixer/mixer.c b/reactos/lib/drivers/sound/mmixer/mixer.c index 46e2c6d8711..4ef2ea9e62a 100644 --- a/reactos/lib/drivers/sound/mmixer/mixer.c +++ b/reactos/lib/drivers/sound/mmixer/mixer.c @@ -65,6 +65,8 @@ MMixerGetCapabilities( MixerCaps->vDriverVersion = MixerInfo->MixCaps.vDriverVersion; MixerCaps->fdwSupport = MixerInfo->MixCaps.fdwSupport; MixerCaps->cDestinations = MixerInfo->MixCaps.cDestinations; + + ASSERT(MixerInfo->MixCaps.szPname[MAXPNAMELEN-1] == 0); wcscpy(MixerCaps->szPname, MixerInfo->MixCaps.szPname); return MM_STATUS_SUCCESS; diff --git a/reactos/lib/drivers/sound/mmixer/priv.h b/reactos/lib/drivers/sound/mmixer/priv.h index 2097294436a..1580c8a99a6 100644 --- a/reactos/lib/drivers/sound/mmixer/priv.h +++ b/reactos/lib/drivers/sound/mmixer/priv.h @@ -178,6 +178,7 @@ MMixerGetTargetPinsByNodeConnectionIndex( IN PKSMULTIPLE_ITEM NodeTypes, IN ULONG bUpDirection, IN ULONG NodeConnectionIndex, + IN ULONG PinCount, OUT PULONG Pins); MIXER_STATUS diff --git a/reactos/lib/drivers/sound/mmixer/sup.c b/reactos/lib/drivers/sound/mmixer/sup.c index 0149df28d43..90b7c0143ee 100644 --- a/reactos/lib/drivers/sound/mmixer/sup.c +++ b/reactos/lib/drivers/sound/mmixer/sup.c @@ -358,7 +358,7 @@ MMixerGetTargetPins( { for(Index = 0; Index < NodeConnectionCount; Index++) { - Status = MMixerGetTargetPinsByNodeConnectionIndex(MixerContext, NodeConnections, NodeTypes, bUpDirection, NodeConnection[Index], Pins); + Status = MMixerGetTargetPinsByNodeConnectionIndex(MixerContext, NodeConnections, NodeTypes, bUpDirection, NodeConnection[Index], PinCount, Pins); ASSERT(Status == STATUS_SUCCESS); } MixerContext->Free((PVOID)NodeConnection); @@ -638,6 +638,7 @@ MMixerGetDeviceName( Status = MixerContext->QueryKeyValue(hKey, L"FriendlyName", (PVOID*)&Name, &Length, &Type); if (Status == MM_STATUS_SUCCESS) { + ASSERT(Length < MAXPNAMELEN); wcscpy(MixerInfo->MixCaps.szPname, Name); MixerContext->Free(Name); return Status; @@ -650,6 +651,7 @@ MMixerGetDeviceName( Status = MixerContext->QueryKeyValue(hKey, L"FriendlyName", (PVOID*)&Name, &Length, &Type); if (Status == MM_STATUS_SUCCESS) { + ASSERT(Length < MAXPNAMELEN); wcscpy(MixerInfo->MixCaps.szPname, Name); MixerContext->Free(Name); } diff --git a/reactos/lib/drivers/sound/mmixer/wave.c b/reactos/lib/drivers/sound/mmixer/wave.c index 0ba0aae41b9..af29f45bb14 100644 --- a/reactos/lib/drivers/sound/mmixer/wave.c +++ b/reactos/lib/drivers/sound/mmixer/wave.c @@ -360,6 +360,8 @@ MMixerInitializeWaveInfo( WaveInfo->DeviceId = MixerData->DeviceId; WaveInfo->PinId = PinId; + // sanity check + ASSERT(wcslen(DeviceName) < MAXPNAMELEN); /* copy device name */ if (bWaveIn) @@ -420,9 +422,6 @@ MMixerInitializeWaveInfo( /* free dataranges buffer */ MixerContext->Free(MultipleItem); - - - if (bWaveIn) { InsertTailList(&MixerList->WaveInList, &WaveInfo->Entry);