Skip to content

Commit ebdb7f2

Browse files
kyleneideckkyleneideck
committed
Fix BGMDriver crash when an app volume is set with no bundle ID.
BGMApp was setting the `kAudioDeviceCustomPropertyAppVolumes` property with only a process ID in the CFDictionary when the app had no bundle ID. BGMDriver would then cause a null deref in `BGM_ClientMap::GetClients`. See #839.
1 parent 478c1fe commit ebdb7f2

4 files changed

Lines changed: 93 additions & 25 deletions

File tree

‎BGMDriver/BGMDriver/BGM_Device.cpp‎

Lines changed: 73 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
// BGM_Device.cpp
1818
// BGMDriver
1919
//
20-
// Copyright © 2016, 2017, 2019 Kyle Neideck
20+
// Copyright © 2016, 2017, 2019, 2025 Kyle Neideck
2121
// Copyright © 2017 Andrew Tonner
2222
// Copyright © 2019 Gordon Childs
2323
// Copyright © 2020 Aleksey Yurkevich
@@ -40,6 +40,7 @@
4040
#include "CADispatchQueue.h"
4141
#include "CAException.h"
4242
#include "CACFArray.h"
43+
#include "CACFDictionary.h"
4344
#include "CACFString.h"
4445
#include "CADebugMacros.h"
4546
#include "CAHostTimeBase.h"
@@ -1001,6 +1002,74 @@ void BGM_Device::Device_GetPropertyData(AudioObjectID inObjectID, pid_t inClient
10011002
};
10021003
}
10031004

1005+
// Validates inAppVolumes for the kAudioDeviceCustomPropertyAppVolumes property as described in
1006+
// BGM_Types.h. Throws CAException(kAudioHardwareIllegalOperationError) if invalid.
1007+
static void ValidateAppVolumesProperty(const CACFArray& inAppVolumes)
1008+
{
1009+
UInt32 theCount = inAppVolumes.GetNumberItems();
1010+
1011+
for(UInt32 i = 0; i < theCount; i++)
1012+
{
1013+
// Each element must be a CFDictionary.
1014+
CFTypeRef theElement = nullptr;
1015+
bool didGetValue = inAppVolumes.GetCFType(i, theElement);
1016+
ThrowIf(!didGetValue || theElement == nullptr,
1017+
CAException(kAudioHardwareIllegalOperationError),
1018+
"BGM_Device::ValidateAppVolumesProperty: Could not get element from array");
1019+
ThrowIf(CFGetTypeID(theElement) != CFDictionaryGetTypeID(),
1020+
CAException(kAudioHardwareIllegalOperationError),
1021+
"BGM_Device::ValidateAppVolumesProperty: Element is not a CFDictionary");
1022+
1023+
CACFDictionary theDict(static_cast<CFDictionaryRef>(theElement), false);
1024+
1025+
// Check for ProcessID. Must be a CFNumber if present.
1026+
CFTypeRef thePIDValue = nullptr;
1027+
bool hasPID = theDict.GetCFType(CFSTR(kBGMAppVolumesKey_ProcessID), thePIDValue);
1028+
if(hasPID && thePIDValue != nullptr)
1029+
{
1030+
ThrowIf(CFGetTypeID(thePIDValue) != CFNumberGetTypeID(),
1031+
CAException(kAudioHardwareIllegalOperationError),
1032+
"BGM_Device::ValidateAppVolumesProperty: ProcessID is not a CFNumber");
1033+
}
1034+
1035+
// Check for BundleID. Must be a CFString if present.
1036+
CFTypeRef theBundleIDValue = nullptr;
1037+
bool hasBundleID = theDict.GetCFType(CFSTR(kBGMAppVolumesKey_BundleID), theBundleIDValue);
1038+
if(hasBundleID && theBundleIDValue != nullptr)
1039+
{
1040+
ThrowIf(CFGetTypeID(theBundleIDValue) != CFStringGetTypeID(),
1041+
CAException(kAudioHardwareIllegalOperationError),
1042+
"BGM_Device::ValidateAppVolumesProperty: BundleID is not a CFString");
1043+
}
1044+
1045+
// At least one of ProcessID or BundleID must be present.
1046+
ThrowIf(!hasPID && !hasBundleID,
1047+
CAException(kAudioHardwareIllegalOperationError),
1048+
"BGM_Device::ValidateAppVolumesProperty: Neither ProcessID nor BundleID present");
1049+
1050+
// Check RelativeVolume. Must be a CFNumber in [0, 100] if present.
1051+
SInt32 theVolume;
1052+
bool hasVolume = theDict.GetSInt32(CFSTR(kBGMAppVolumesKey_RelativeVolume), theVolume);
1053+
if(hasVolume)
1054+
{
1055+
ThrowIf(theVolume < kAppRelativeVolumeMinRawValue ||
1056+
theVolume > kAppRelativeVolumeMaxRawValue,
1057+
CAException(kAudioHardwareIllegalOperationError),
1058+
"BGM_Device::ValidateAppVolumesProperty: RelativeVolume out of range");
1059+
}
1060+
1061+
// Check PanPosition. Must be a CFNumber in [-100, 100] if present.
1062+
SInt32 thePan;
1063+
bool hasPan = theDict.GetSInt32(CFSTR(kBGMAppVolumesKey_PanPosition), thePan);
1064+
if(hasPan)
1065+
{
1066+
ThrowIf(thePan < kAppPanLeftRawValue || thePan > kAppPanRightRawValue,
1067+
CAException(kAudioHardwareIllegalOperationError),
1068+
"BGM_Device::ValidateAppVolumesProperty: PanPosition out of range");
1069+
}
1070+
}
1071+
}
1072+
10041073
void BGM_Device::Device_SetPropertyData(AudioObjectID inObjectID, pid_t inClientPID, const AudioObjectPropertyAddress& inAddress, UInt32 inQualifierDataSize, const void* inQualifierData, UInt32 inDataSize, const void* inData)
10051074
{
10061075
switch(inAddress.mSelector)
@@ -1092,6 +1161,8 @@ void BGM_Device::Device_SetPropertyData(AudioObjectID inObjectID, pid_t inClient
10921161

10931162
CACFArray array(arrayRef, false);
10941163

1164+
ValidateAppVolumesProperty(array);
1165+
10951166
bool propertyWasChanged = false;
10961167

10971168
CAMutex::Locker theStateLocker(mStateMutex);
@@ -1104,7 +1175,7 @@ void BGM_Device::Device_SetPropertyData(AudioObjectID inObjectID, pid_t inClient
11041175
{
11051176
Throw(CAException(kAudioHardwareIllegalOperationError));
11061177
}
1107-
1178+
11081179
if(propertyWasChanged)
11091180
{
11101181
// Send notification

‎BGMDriver/BGMDriver/DeviceClients/BGM_ClientMap.cpp‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
// BGM_ClientMap.cpp
1818
// BGMDriver
1919
//
20-
// Copyright © 2016, 2017, 2019 Kyle Neideck
20+
// Copyright © 2016, 2017, 2019, 2025 Kyle Neideck
2121
// Copyright © 2017 Andrew Tonner
2222
//
2323

@@ -264,6 +264,9 @@ std::vector<BGM_Client*> * _Nullable BGM_ClientMap::GetClients(pid_t inAppPid) {
264264
}
265265

266266
std::vector<BGM_Client*> * _Nullable BGM_ClientMap::GetClients(CACFString inAppBundleID) {
267+
if(!inAppBundleID.IsValid()) {
268+
return nullptr;
269+
}
267270
return GetClientsFromMap(mClientMapByBundleIDShadow, inAppBundleID);
268271
}
269272

@@ -335,12 +338,12 @@ bool BGM_ClientMap::SetClientsRelativeVolume(pid_t searchKey, Float32 inRelative
335338
bool BGM_ClientMap::SetClientsRelativeVolume(CACFString searchKey, Float32 inRelativeVolume)
336339
{
337340
bool didChangeVolume = false;
338-
341+
339342
CAMutex::Locker theShadowMapsLocker(mShadowMapsMutex);
340-
343+
341344
auto theSetVolumesInShadowMapsFunc = [&] {
342345
// Look up the clients for the key and update their volumes
343-
346+
344347
auto theClients = GetClients(searchKey);
345348
if(theClients != nullptr)
346349
{
@@ -389,9 +392,9 @@ bool BGM_ClientMap::SetClientsPanPosition(pid_t searchKey, SInt32 inPanPosition)
389392
bool BGM_ClientMap::SetClientsPanPosition(CACFString searchKey, SInt32 inPanPosition)
390393
{
391394
bool didChangePanPosition = false;
392-
395+
393396
CAMutex::Locker theShadowMapsLocker(mShadowMapsMutex);
394-
397+
395398
auto theSetPansInShadowMapsFunc = [&] {
396399
// Look up the clients for the key and update their pan positions
397400
auto theClients = GetClients(searchKey);

‎BGMDriver/BGMDriver/DeviceClients/BGM_ClientMap.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
// BGM_ClientMap.h
1818
// BGMDriver
1919
//
20-
// Copyright © 2016 Kyle Neideck
20+
// Copyright © 2016, 2025 Kyle Neideck
2121
//
2222

2323
#ifndef __BGMDriver__BGM_ClientMap__
@@ -131,11 +131,13 @@ class BGM_ClientMap
131131
// Returns true if a client for PID inAppPID was found and its relative volume changed.
132132
bool SetClientsRelativeVolume(pid_t inAppPID, Float32 inRelativeVolume);
133133
// Returns true if a client for bundle ID inAppBundleID was found and its relative volume changed.
134+
// inAppBundleID may contain a null CFStringRef, in which case it returns false.
134135
bool SetClientsRelativeVolume(CACFString inAppBundleID, Float32 inRelativeVolume);
135136

136137
// Returns true if a client for PID inAppPID was found and its pan position changed.
137138
bool SetClientsPanPosition(pid_t inAppPID, SInt32 inPanPosition);
138139
// Returns true if a client for bundle ID inAppBundleID was found and its pan position changed.
140+
// inAppBundleID may contain a null CFStringRef, in which case it returns false.
139141
bool SetClientsPanPosition(CACFString inAppBundleID, SInt32 inPanPosition);
140142

141143
void StartIONonRT(UInt32 inClientID) { UpdateClientIOStateNonRT(inClientID, true); }

‎BGMDriver/BGMDriver/DeviceClients/BGM_Clients.cpp‎

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
// BGM_Clients.cpp
1818
// BGMDriver
1919
//
20-
// Copyright © 2016, 2017, 2019 Kyle Neideck
20+
// Copyright © 2016, 2017, 2019, 2025 Kyle Neideck
2121
// Copyright © 2017 Andrew Tonner
2222
//
2323

@@ -26,6 +26,7 @@
2626

2727
// Local Includes
2828
#include "BGM_Types.h"
29+
#include "BGM_Utils.h"
2930
#include "BGM_PlugIn.h"
3031

3132
// PublicUtility Includes
@@ -324,26 +325,21 @@ bool BGM_Clients::SetClientsRelativeVolumes(const CACFArray inAppVolumes)
324325
// Get the app's PID from the dict
325326
pid_t theAppPID;
326327
bool didFindPID = theAppVolume.GetSInt32(CFSTR(kBGMAppVolumesKey_ProcessID), theAppPID);
327-
328+
328329
// Get the app's bundle ID from the dict
329330
CACFString theAppBundleID;
330331
theAppBundleID.DontAllowRelease();
331332
theAppVolume.GetCACFString(CFSTR(kBGMAppVolumesKey_BundleID), theAppBundleID);
332-
333-
ThrowIf(!didFindPID && !theAppBundleID.IsValid(),
334-
BGM_InvalidClientRelativeVolumeException(),
335-
"BGM_Clients::SetClientsRelativeVolumes: App volume was sent without PID or bundle ID for app");
336-
333+
334+
BGMAssert(didFindPID || theAppBundleID.IsValid(),
335+
"BGM_Clients::SetClientsRelativeVolumes: No PID or bundle ID");
336+
337337
bool didGetVolume;
338338
{
339339
SInt32 theRawRelativeVolume;
340340
didGetVolume = theAppVolume.GetSInt32(CFSTR(kBGMAppVolumesKey_RelativeVolume), theRawRelativeVolume);
341-
341+
342342
if (didGetVolume) {
343-
ThrowIf(didGetVolume && (theRawRelativeVolume < kAppRelativeVolumeMinRawValue || theRawRelativeVolume > kAppRelativeVolumeMaxRawValue),
344-
BGM_InvalidClientRelativeVolumeException(),
345-
"BGM_Clients::SetClientsRelativeVolumes: Relative volume for app out of valid range");
346-
347343
// Apply the volume curve to the raw volume
348344
//
349345
// mRelativeVolumeCurve uses the default kPow2Over1Curve transfer function, so we also multiply by 4 to
@@ -372,10 +368,6 @@ bool BGM_Clients::SetClientsRelativeVolumes(const CACFArray inAppVolumes)
372368
SInt32 thePanPosition;
373369
didGetPanPosition = theAppVolume.GetSInt32(CFSTR(kBGMAppVolumesKey_PanPosition), thePanPosition);
374370
if (didGetPanPosition) {
375-
ThrowIf(didGetPanPosition && (thePanPosition < kAppPanLeftRawValue || thePanPosition > kAppPanRightRawValue),
376-
BGM_InvalidClientPanPositionException(),
377-
"BGM_Clients::SetClientsRelativeVolumes: Pan position for app out of valid range");
378-
379371
if(mClientMap.SetClientsPanPosition(theAppPID, thePanPosition))
380372
{
381373
didChangeAppVolumes = true;

0 commit comments

Comments
 (0)