search

mackron / dr_libs

Audio decoding libraries for C/C++, each in a single source file.
Other
1.27k stars 206 forks source link

dr_flac: The 'pFlac' pointer was used unsafely after it was verified against nullptr. #97

Closed kcgen closed 4 years ago

kcgen commented 4 years ago

image

https://www.viva64.com/en/w/v1004/

kcgen commented 4 years ago

Checking the pFlac pointer first-thing in the macro solves this.

static type* drflac__full_read_and_close_ ## extension ... 
{ \
    if (pFlac == NULL) { \
        goto on_error; \
    } \
<...>
mackron commented 4 years ago

I'm not seeing the issue here. I clearly have an assert in place that checks if pFlac is NULL here: https://github.com/mackron/dr_libs/blob/0f1fd833b8b9948869b6b3547ac62a33412aaa1b/dr_flac.h#L10367

I don't want to blindly throw a run time check in there because that is an internal API which should always have a valid pFlac object passed into it. If it doesn't it means there's a bug at a higher level and I want that assert to get triggered. I also checked all the occurances of drflac__full_read_and_close_*() and they are all guarding against the NULL case just fine. Just not seeing this one... Though that doesn't mean there isn't an issue somewhere that I'm just missing...

kcgen commented 4 years ago

I agree Dave; and with PVS not providing the path to prove this, my guess is it simply loses the call thread across the macro boundary (or something like that, and just isn't seeing your prior checks). Will add this to the ignore file.

mackron commented 4 years ago

Cool. I'll close this one and set it to will not fix.