m1dugh / native-sound-mixer

MIT License
25 stars 13 forks source link

Add more HRESULT checks to fix null pointer dereferences #45

Closed JJ-8 closed 7 months ago

JJ-8 commented 7 months ago

Hello again!

Thanks again for this very useful project! After the merge of https://github.com/m1dugh/native-sound-mixer/issues/42, that issue went away, but new null pointer dereference issues appeared in our bug tracker. I had a look at it and found the following:

On multiple places, there is an interaction with endpointVolume and the function is expected to return something, but the return value of endpointVolume is not checked. Therefore, undefined values may be returned. This PR adds return value checks with default values as return values for failing endpointVolume interactions. This should fix null pointer dereference issues.

For the record, here is the stack trace that I initially triaged:

EXCEPTION_ACCESS_VIOLATION_READ: Attempted to dereference null pointer.

0  win-sound-mixer.node +0xcc6a  0xcc6a
   r10 = 0xfffefa1f215        rbp = 0x0              
   r11 = 0xaa8a88282a288a22   rbx = 0x22517d328c8    
   r12 = 0x22517dfc2e0        rcx = 0x0              
   r13 = 0x8e1cffcbb8         rdi = 0x22517d32880    
   r14 = 0xf                  rdx = 0x7fff7d23b340   
   r15 = 0x0                  rip = 0x7fff7cffcc6a   
    r8 = 0x1                  rsi = 0x8e1cffca50     
    r9 = 0x1                  rsp = 0x8e1cffc9c0     
   rax = 0x80070000           
1  ntdll.dll +0x3dcb4            0x3dcb5
2  win-sound-mixer.node +0x10d7b 0x10d7c
3  win-sound-mixer.node +0xe386  0xe387
4  win-sound-mixer.node +0x733b  0x733c

Instruction with the issue: mov rax, [rcx]. rcx is clearly NULL and therefore can't be dereferenced.

The stack trace points to the following lines of code: https://github.com/m1dugh/native-sound-mixer/blob/master/cppsrc/win/win-sound-mixer.cpp#L235-L236:

    _oldVolume = GetVolume();
    _oldMute = (BOOL)GetMute();

Please note that I still don't have a setup to compile and test this myself and therefore the code is NOT tested. @m1dugh can you have a look at it?

I was not able to reproduce the error myself, but maybe you can try disabling your audio card in the Windows settings to create a faulty audio device that may trigger these errors.

m1dugh commented 7 months ago

Merged ! Thank you for your work !