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.

Sign up for GitHub

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

Fix multiple ubsan crashes #42

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix multiple ubsan crashes #42

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c48e4c6
Always check the number of coefficients
antlarr Mar 6, 2017
beacc44
Check for multiplication overflow in MSADPCM decodeSample
antlarr Mar 6, 2017
7d65f89
Check for multiplication overflow in sfconvert
antlarr Mar 6, 2017
ce536d7
Fix signature of multiplyCheckOverflow. It returns a bool, not an int
antlarr Mar 10, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions libaudiofile/WAVE.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,12 @@ status WAVEFile::parseFormat(const Tag &id, uint32_t size)

/* numCoefficients should be at least 7. */
assert(numCoefficients >= 7 && numCoefficients <= 255);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the assert statement still needed in this case?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, in case the code is built with DEBUG defined (as I guess @mpruett does) the assert would fail and I didn't want to change that behavior for the main developer. Also, in the general case (when DEBUG is not defined, as most distributions build this code), the assert is already a NOP.

if (numCoefficients < 7 || numCoefficients > 255)
{
_af_error(AF_BAD_HEADER,
"Bad number of coefficients");
return AF_FAIL;
}

m_msadpcmNumCoefficients = numCoefficients;

Expand Down