mackron / dr_libs

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

dr_wav 0.13.2: PVS Studio flagged memcpy underflow of pointer #205

Closed kcgen closed 2 years ago

kcgen commented 2 years ago

2021-10-02_10-16

Open attached in browser, search list for dr_wav:

dr_wav-0.13.2-pvs-issues.tar.gz

mackron commented 2 years ago

I'm not 100% sure what would be causing this, but I did fix in bug in some lines just above that where pReadPointer wasn't being read properly. I'm wondering if there might be a small chance that might fix this error? Are you able to try that again? It's in the dev branch.

kcgen commented 2 years ago

Latest PVS results: https://github.com/dosbox-staging/dosbox-staging/suites/3969999611/artifacts/99800840; unfortunately looks to the be same.

mackron commented 2 years ago

I've pushed a potential fix to the dev branch. Are you able to give that another run? If this doesn't fix it, I'm calling this as a false positive.

kcgen commented 2 years ago

Attached is the updated scan; thank you for continuing to fix these newly flagged issues.

pvs-analysis-report.zip

I've posted it in browsable form here, if you sort by location (temporarily):

https://kcgen.duckdns.org/pvs-report-2021-11-22_T0941-b27001bb/general/

mackron commented 2 years ago

That build is one commit out of date. I think you took the version from the other issue just before I pushed the update for this one. In any case, I've pushed another change to clean up those new V590 warnings. That V547 warning is a wontfix because it's a valid statement for the 32-bit build. It's just that V512 memcpy() warning we need to fix for this one.

Thanks for rerunning these for me.

kcgen commented 2 years ago

Thanks for the heads up; I've added that suppression for the 32-bit scenario, and the list is down significantly!

Customers Specific | dr_wav.h:3104 | High | V2006 | Implicit type conversion from enum type to integer type.
Customers Specific | dr_wav.h:3049 | High | V2006 | Implicit type conversion from enum type to integer type.
General Analysis | dr_wav.h:2368 | Medium | V512 | A call of the 'memcpy' function will lead to underflow of the buffer 'pReadPointer'.

Here's the pvs-analysis-report.zip

mackron commented 2 years ago

OK, something has gone wrong with your local copy of dr_wav. I have updated the dev branch, but you're missing at least one commit. The error on line 2368 is not consistent with the current version. See line 2368 in the dev branch here: https://github.com/mackron/dr_libs/blob/dev/dr_wav.h#L2368.

I completely refactored that pReadPointer thing in this commit. I think you're missing at least this commit: https://github.com/mackron/dr_libs/commit/39e1a2b89569fe71740cea722121cb312fd97d08#diff-87b24e5d4549a6fa9432d243bfc2f4c5b7a86298f1c0e3c8af26ab8c5898b066

kcgen commented 2 years ago

Thanks for catching this. Will get the latest rescanned first-thing in the morning.

kcgen commented 2 years ago

That's takes care of all of them!

pvs-analysis-report.zip

Thanks for these polishing touches, @mackron!

mackron commented 2 years ago

Thanks for confirming that for me. This has been released.