search

dbry / WavPack

WavPack encode/decode library, command-line programs, and several plugins
BSD 3-Clause "New" or "Revised" License
363 stars 66 forks source link

Review of past security vulnerabilites wrt v4.70.0 #94

Closed utkarsh2102 closed 3 years ago

utkarsh2102 commented 3 years ago

Hello @dbry,

I am incredibly thankful for your help for CVE-2020-35738 wrt 4.70.0. As mentioned there already, I am fixing all the pending security vulnerabilities for Debian ELTS which is at version 4.70.0.

I have carefully reviewed and backported all that I could find. Here's my take on this:

CVE ID Fixing Commit Backported?
CVE-2016-10169 https://github.com/dbry/WavPack/commit/4bc05fc490b66ef2d45b1de26abf1455b486b0dc Only a part of it, i.e., the hunk in src/read_words.c of the fixing commit. The initial hunk wasn't a part of the older version.
CVE-2018-19840 https://github.com/dbry/WavPack/commit/070ef6f138956d9ea9612e69586152339dbefe51 Backported completely.
CVE-2018-19841 https://github.com/dbry/WavPack/commit/bba5389dc598a92bdf2b297c3ea34620b6679b5b My assessment was the v4.70.0 was not affected as I couldn't find the vulnerable code. Let me know if this isn't the case.
CVE-2019-1010315 https://github.com/dbry/WavPack/commit/4c0faba32fddbd0745cbfaf1e1aeb3da5d35b9fc Vulnerable code is not present; DFF support was introduced later.
CVE-2019-1010317 https://github.com/dbry/WavPack/commit/f68a9555b548306c5b1ee45199ccdc4a16a6101b Vulnerable code is not present; CAF support was introduced later.
CVE-2019-1010319 https://github.com/dbry/WavPack/commit/33a0025d1d63ccd05d9dbaa6923d52b1446a62fe It looks like CLEAR (WaveHeader); was already in v4.70.0. So I guess this is not affecting that version. Right?
CVE-2019-11498 https://github.com/dbry/WavPack/commit/bc6cba3f552c44565f7f1e66dc1580189addb2b4 Vulnerable code is not present; DFF support was introduced later.
CVE-2020-35738 https://github.com/dbry/WavPack/commit/63f3ec70129843dd64e11aa4c21c4a1cf00c9f1c and https://github.com/dbry/WavPack/commit/89df160596132e3bd666322e1c20b2ebd4b92cd0 Backported already with your help! ❤️

If you have some time (and can take a quick look at this), could you help me verify that this is indeed correct? Based on this, I'll also backport to v5.0.0, where some of these are already fixed.

dbry commented 3 years ago

All of your assumptions are correct.

Clarity on CVE-2018-19840 is that that change is actually a subset of the very recent fix for CVE-2020-35738, so it really shouldn't be its own patch (i.e. there's no reason to check for zero, then check for less than or equal to zero).

On CVE-2019-1010319, that is totally new code that doesn't exist before 5.0. The CLEAR (WaveHeader) you are referring to in 4.70 now resides the file cli/riff.c. But yes, no change required.

Thanks so much for attending to this!

utkarsh2102 commented 3 years ago

Hello 👋🏻

Clarity on CVE-2018-19840 is that that change is actually a subset of the very recent fix for CVE-2020-35738, so it really shouldn't be its own patch (i.e. there's no reason to check for zero, then check for less than or equal to zero).

Oh yes! I just marked both as fixed with the recent patch applied! Thank you, though!

On CVE-2019-1010319, that is totally new code that doesn't exist before 5.0. The CLEAR (WaveHeader) you are referring to in 4.70 now resides the file cli/riff.c. But yes, no change required.

Perfect, thank you!

Thanks so much for attending to this!

No, thank you so much for your review and great help on this! ❤️