Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more HRESULT checks to fix null pointer dereferences #45

Merged
merged 2 commits into from
Mar 2, 2024

Conversation

JJ-8
Copy link
Contributor

@JJ-8 JJ-8 commented Mar 2, 2024

Hello again!

Thanks again for this very useful project! After the merge of #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.

JJ-8 added 2 commits March 2, 2024 17:31
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 commit adds return value checks with default values as return values for failing endpointVolume interactions.
This should fix null pointer dereference issues.
@m1dugh m1dugh enabled auto-merge (rebase) March 2, 2024 19:40
@m1dugh m1dugh merged commit ebdc89a into m1dugh:master Mar 2, 2024
6 checks passed
@m1dugh
Copy link
Owner

m1dugh commented Mar 2, 2024

Merged !
Thank you for your work !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants