Undefined behavior / float-cast-overflow in speex_decode_stereo_int

[dklimkin]

speex_decode_stereo_int:276-277

      data[2*i] = (spx_int16_t)MULT16_16_P14(stereo->smooth_left, tmp);
      data[2*i+1] = (spx_int16_t)MULT16_16_P14(stereo->smooth_right, tmp);

casts float to 16 bit int which can cause an overflow and UB. This UB typically results in SIGILL with clang.

[Dutchman101]

Hello @tmatth, i made PR #28 to fix this

[tmatth]

speex_decode_stereo_int:276-277

      data[2*i] = (spx_int16_t)MULT16_16_P14(stereo->smooth_left, tmp);
      data[2*i+1] = (spx_int16_t)MULT16_16_P14(stereo->smooth_right, tmp);

casts float to 16 bit int which can cause an overflow and UB. This UB typically results in SIGILL with clang.

Do you have a POC? Also I'm assuming you're talking about the inside of the MULT16_16_P14 calls and not the spx_int16_t casts of the results which should be fine.

[tmatth]

Hello @tmatth, i made PR #28 to fix this

Thanks I'll review there, although this is just avoiding undefined behavior in the fixed-debug builds so I don't see how that would fix this issue.

[Dutchman101]

Hello @tmatth, i made PR #28 to fix this

Thanks I'll review there, although this is just avoiding undefined behavior in the fixed-debug builds so I don't see how that would fix this issue.

@tmatth

MULT16_16_P14 is created in fixed_debug, so i figured that's where to fix it. Am i wrong?

Also, now that i have your attention, would you please take a look at what i feel may be a critical security bug? PR #29: Fix OOB

[tmatth]

MULT16_16_P14 is created in fixed_debug, so i figured that's where to fix it. Am i wrong?

That's the version that gets used if you ./configure --enable-fixed-point-debug (only really used when debugging). The version defined in fixed_generic also needs to be fixed (as that is what will be used by normal fixed-point builds).