From aaf333862fb54d4cfc19a1866b76500d86679719 Mon Sep 17 00:00:00 2001 From: Ondrej Holy Date: Tue, 28 Apr 2026 04:41:15 +0000 Subject: [PATCH] [codec,dsp] fix array bounds checks Backport of commit 16df2300e1e3f5a51f68fb1626429e58b531b7c8. Adapted for 2.11.x: uses C89 declaration style (variables declared at top of scope, new variables wrapped in blocks), no `WINPR_RESTRICT`, kept `int channel` in decode function, `dsp_encode_ima_adpcm_sample` keeps `int channel`. Made-with: Cursor --- libfreerdp/codec/dsp.c | 91 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 82 insertions(+), 9 deletions(-) diff --git a/libfreerdp/codec/dsp.c b/libfreerdp/codec/dsp.c index e79911d..6d4f780 100644 --- a/libfreerdp/codec/dsp.c +++ b/libfreerdp/codec/dsp.c @@ -297,7 +297,16 @@ static UINT16 dsp_decode_ima_adpcm_sample(ADPCM* adpcm, unsigned int channel, BY { INT32 ss; INT32 d; - ss = ima_step_size_table[adpcm->ima.last_step[channel]]; + INT16 offset; + + WINPR_ASSERT(channel < ARRAYSIZE(adpcm->ima.last_step)); + WINPR_ASSERT(channel < ARRAYSIZE(adpcm->ima.last_sample)); + + offset = adpcm->ima.last_step[channel]; + WINPR_ASSERT(offset >= 0); + WINPR_ASSERT(offset < ARRAYSIZE(ima_step_size_table)); + + ss = ima_step_size_table[offset]; d = (ss >> 3); if (sample & 1) @@ -320,6 +329,8 @@ static UINT16 dsp_decode_ima_adpcm_sample(ADPCM* adpcm, unsigned int channel, BY d = 32767; adpcm->ima.last_sample[channel] = (INT16)d; + + WINPR_ASSERT(sample < ARRAYSIZE(ima_step_index_table)); adpcm->ima.last_step[channel] += ima_step_index_table[sample]; if (adpcm->ima.last_step[channel] < 0) @@ -368,6 +379,9 @@ static BOOL freerdp_dsp_decode_ima_adpcm(FREERDP_DSP_CONTEXT* context, const BYT { if (size % block_size == 0) { + if (size < 4) + return FALSE; + context->adpcm.ima.last_sample[0] = (INT16)(((UINT16)(*src)) | (((UINT16)(*(src + 1))) << 8)); context->adpcm.ima.last_step[0] = (INT16)(*(src + 2)); @@ -377,6 +391,8 @@ static BOOL freerdp_dsp_decode_ima_adpcm(FREERDP_DSP_CONTEXT* context, const BYT if (channels > 1) { + if (size < 4) + return FALSE; context->adpcm.ima.last_sample[1] = (INT16)(((UINT16)(*src)) | (((UINT16)(*(src + 1))) << 8)); context->adpcm.ima.last_step[1] = (INT16)(*(src + 2)); @@ -388,6 +404,8 @@ static BOOL freerdp_dsp_decode_ima_adpcm(FREERDP_DSP_CONTEXT* context, const BYT if (channels > 1) { + if (size < 8) + return FALSE; for (i = 0; i < 8; i++) { channel = (i < 4 ? 0 : 1); @@ -407,6 +425,8 @@ static BOOL freerdp_dsp_decode_ima_adpcm(FREERDP_DSP_CONTEXT* context, const BYT } else { + if (size < 1) + return FALSE; sample = ((*src) & 0x0f); decoded = dsp_decode_ima_adpcm_sample(&context->adpcm, 0, sample); *dst++ = (decoded & 0xFF); @@ -687,7 +707,16 @@ static BYTE dsp_encode_ima_adpcm_sample(ADPCM* adpcm, int channel, INT16 sample) INT32 ss; BYTE enc; INT32 diff; - ss = ima_step_size_table[adpcm->ima.last_step[channel]]; + INT16 offset; + + WINPR_ASSERT(channel < ARRAYSIZE(adpcm->ima.last_step)); + WINPR_ASSERT(channel < ARRAYSIZE(adpcm->ima.last_sample)); + + offset = adpcm->ima.last_step[channel]; + WINPR_ASSERT(offset >= 0); + WINPR_ASSERT(offset < ARRAYSIZE(ima_step_size_table)); + + ss = ima_step_size_table[offset]; d = e = sample - adpcm->ima.last_sample[channel]; diff = ss >> 3; enc = 0; @@ -733,6 +762,8 @@ static BYTE dsp_encode_ima_adpcm_sample(ADPCM* adpcm, int channel, INT16 sample) diff = 32767; adpcm->ima.last_sample[channel] = (INT16)diff; + + WINPR_ASSERT(enc < ARRAYSIZE(ima_step_index_table)); adpcm->ima.last_step[channel] += ima_step_index_table[enc]; if (adpcm->ima.last_step[channel] < 0) @@ -847,10 +878,24 @@ static INLINE INT16 freerdp_dsp_decode_ms_adpcm_sample(ADPCM* adpcm, BYTE sample { INT8 nibble; INT32 presample; + BYTE predictor; + INT32 coeff1 = 0; + INT32 coeff2 = 0; + + WINPR_ASSERT(channel < ARRAYSIZE(adpcm->ms.sample1)); + WINPR_ASSERT(channel < ARRAYSIZE(adpcm->ms.sample2)); + WINPR_ASSERT(channel < ARRAYSIZE(adpcm->ms.delta)); + WINPR_ASSERT(channel < ARRAYSIZE(adpcm->ms.predictor)); + nibble = (sample & 0x08 ? (INT8)sample - 16 : (INT8)sample); - presample = ((adpcm->ms.sample1[channel] * ms_adpcm_coeffs1[adpcm->ms.predictor[channel]]) + - (adpcm->ms.sample2[channel] * ms_adpcm_coeffs2[adpcm->ms.predictor[channel]])) / - 256; + predictor = adpcm->ms.predictor[channel]; + if (predictor < ARRAYSIZE(ms_adpcm_coeffs1)) + coeff1 = ms_adpcm_coeffs1[predictor]; + + if (predictor < ARRAYSIZE(ms_adpcm_coeffs2)) + coeff2 = ms_adpcm_coeffs2[predictor]; + presample = + ((adpcm->ms.sample1[channel] * coeff1) + (adpcm->ms.sample2[channel] * coeff2)) / 256; presample += nibble * adpcm->ms.delta[channel]; if (presample > 32767) @@ -860,7 +905,14 @@ static INLINE INT16 freerdp_dsp_decode_ms_adpcm_sample(ADPCM* adpcm, BYTE sample adpcm->ms.sample2[channel] = adpcm->ms.sample1[channel]; adpcm->ms.sample1[channel] = presample; - adpcm->ms.delta[channel] = adpcm->ms.delta[channel] * ms_adpcm_adaptation_table[sample] / 256; + + { + INT32 tableval = 0; + if (sample < ARRAYSIZE(ms_adpcm_adaptation_table)) + tableval = ms_adpcm_adaptation_table[sample]; + + adpcm->ms.delta[channel] = adpcm->ms.delta[channel] * tableval / 256; + } if (adpcm->ms.delta[channel] < 16) adpcm->ms.delta[channel] = 16; @@ -891,6 +943,9 @@ static BOOL freerdp_dsp_decode_ms_adpcm(FREERDP_DSP_CONTEXT* context, const BYTE { if (channels > 1) { + if (size < 14) + return FALSE; + context->adpcm.ms.predictor[0] = *src++; context->adpcm.ms.predictor[1] = *src++; context->adpcm.ms.delta[0] = read_int16(src); @@ -917,6 +972,9 @@ static BOOL freerdp_dsp_decode_ms_adpcm(FREERDP_DSP_CONTEXT* context, const BYTE } else { + if (size < 7) + return FALSE; + context->adpcm.ms.predictor[0] = *src++; context->adpcm.ms.delta[0] = read_int16(src); src += 2; @@ -934,12 +992,16 @@ static BOOL freerdp_dsp_decode_ms_adpcm(FREERDP_DSP_CONTEXT* context, const BYTE if (channels > 1) { + if (size < 1) + return FALSE; sample = *src++; size--; write_int16(dst, freerdp_dsp_decode_ms_adpcm_sample(&context->adpcm, sample >> 4, 0)); dst += 2; write_int16(dst, freerdp_dsp_decode_ms_adpcm_sample(&context->adpcm, sample & 0x0F, 1)); dst += 2; + if (size < 1) + return FALSE; sample = *src++; size--; write_int16(dst, freerdp_dsp_decode_ms_adpcm_sample(&context->adpcm, sample >> 4, 0)); @@ -949,6 +1011,8 @@ static BOOL freerdp_dsp_decode_ms_adpcm(FREERDP_DSP_CONTEXT* context, const BYTE } else { + if (size < 1) + return FALSE; sample = *src++; size--; write_int16(dst, freerdp_dsp_decode_ms_adpcm_sample(&context->adpcm, sample >> 4, 0)); @@ -962,10 +1026,16 @@ static BOOL freerdp_dsp_decode_ms_adpcm(FREERDP_DSP_CONTEXT* context, const BYTE return TRUE; } -static BYTE freerdp_dsp_encode_ms_adpcm_sample(ADPCM* adpcm, INT32 sample, int channel) +static BYTE freerdp_dsp_encode_ms_adpcm_sample(ADPCM* adpcm, INT32 sample, size_t channel) { INT32 presample; INT32 errordelta; + + WINPR_ASSERT(channel < ARRAYSIZE(adpcm->ms.sample1)); + WINPR_ASSERT(channel < ARRAYSIZE(adpcm->ms.sample2)); + WINPR_ASSERT(channel < ARRAYSIZE(adpcm->ms.delta)); + WINPR_ASSERT(channel < ARRAYSIZE(adpcm->ms.predictor)); + presample = ((adpcm->ms.sample1[channel] * ms_adpcm_coeffs1[adpcm->ms.predictor[channel]]) + (adpcm->ms.sample2[channel] * ms_adpcm_coeffs2[adpcm->ms.predictor[channel]])) / 256; @@ -988,8 +1058,11 @@ static BYTE freerdp_dsp_encode_ms_adpcm_sample(ADPCM* adpcm, INT32 sample, int c adpcm->ms.sample2[channel] = adpcm->ms.sample1[channel]; adpcm->ms.sample1[channel] = presample; - adpcm->ms.delta[channel] = - adpcm->ms.delta[channel] * ms_adpcm_adaptation_table[(((BYTE)errordelta) & 0x0F)] / 256; + { + const size_t offset = (((BYTE)errordelta) & 0x0F); + WINPR_ASSERT(offset < ARRAYSIZE(ms_adpcm_adaptation_table)); + adpcm->ms.delta[channel] = adpcm->ms.delta[channel] * ms_adpcm_adaptation_table[offset] / 256; + } if (adpcm->ms.delta[channel] < 16) adpcm->ms.delta[channel] = 16; -- 2.53.0