m1dugh / native-sound-mixer

MIT License
25 stars 13 forks source link

EXCEPTION_ACCESS_VIOLATION_READ: Attempted to dereference null pointer #42

Closed JJ-8 closed 8 months ago

JJ-8 commented 8 months ago

Hi, first of all thank you for this very useful project :)

I think I have found a bug in the source code which causes a null pointer dereference:

    device->Activate(__uuidof(IAudioEndpointVolume), CLSCTX_ALL, NULL,
        (LPVOID *)&endpointVolume);

    if (device_cb == NULL)
        device_cb = new SoundMixerAudioEndpointVolumeCallback(this);
    endpointVolume->RegisterControlChangeNotify(device_cb);

    return true;

From: https://github.com/m1dugh/native-sound-mixer/blob/master/cppsrc/win/win-sound-mixer.cpp#L308-L315

According to https://learn.microsoft.com/en-us/windows/win32/api/mmdeviceapi/nf-mmdeviceapi-immdevice-activate the endpointVolume could be NULL when the Activate call fails. However, the return value is not checked and later endpointVolume->RegisterControlChangeNotify(device_cb); is done. When endpointVolume is NULL, this causes a null pointer dereference. I have no test case to reproduce this behavior because I am only seeing this from production users by collecting crashes. In case it might be useful for you, here is a snippet of the native crash dump:

EXCEPTION_ACCESS_VIOLATION_READ: Attempted to dereference null pointer.

0  win-sound-mixer.node +0xeaab  0xeaab
   r10 = 0x5555555555555555   rbp = 0x1b5a0af79f0    
   r11 = 0xaaaa88a8c00aa2aa   rbx = 0x1b5a0a54d70    
   r12 = 0x1b5a0ade5e0        rcx = 0x0              
   r13 = 0x497fffc3b8         rdi = 0x1b5a0a54d00    
   r14 = 0x1b55d6b4250        rdx = 0x1b55d681b70    
   r15 = 0x0                  rip = 0x7ffb654aeaab   
    r8 = 0x0                  rsi = 0x1b5a0a54d18    
    r9 = 0x555577573ff55d55   rsp = 0x497fffc110     
   rax = 0x7ffb654cdbe0       
1  win-sound-mixer.node +0xc95b  0xc95c
2  ntdll.dll +0x3dca4            0x3dca5
3  win-sound-mixer.node +0x10beb 0x10bec
4  win-sound-mixer.node +0xe056  0xe057
5  win-sound-mixer.node +0x728b  0x728c
6  win-sound-mixer.node +0x31fd  0x31fe
7  win-sound-mixer.node +0x2767  0x2768

This is the instruction on where it fails: mov rax, [rcx]. You can clearly see that rcx is set to 0x0 which being dereferenced.

I don't know the code well enough to know how to handle this case. Do you maybe now why Activate could fail?

m1dugh commented 8 months ago

Hello,

I don't know how activation could fail, however, I will fix the bug and deploy the new package soon, unless you want to make a PR with the fix that I will be happy to accept.

Thanks

JJ-8 commented 8 months ago

Hi @m1dugh, thank you for the quick response! It would be great if you could fix this issue since then I don't have to setup the development environment :)