[FFmpeg-devel] [PATCH] libavcodec/g723_1enc: Fix crash

[FFmpeg-devel] [PATCH] libavcodec/g723_1enc: Fix crash

[Kovacs, Zsolt via ffmpeg-devel]

[-- Attachment #1: Type: text/plain, Size: 2170 bytes --]

Hi All,

We had crashes in prod while compressing audio with G.723.1 using ffmpeg as a library. The callstack was:

fcb_search, line 1027
ff_encode_encode_cb, line 253
encode_simple_internal, line 339
avcodec_send_frame, line 530

After debugging the issue I found that the crash is caused by FCBParam optim; being uninitialized in fcb_search().

The context of fcb_search(), line 1027 in libavcodec\g723_1enc.c:
    /* Reconstruct the excitation */
    memset(buf, 0, sizeof(int16_t) * SUBFRAME_LEN);
    for (i = 0; i < pulse_cnt; i++)
        buf[optim.pulse_pos[i]] = optim.pulse_sign[i];

The last line is 1027, the crash is caused by out of bounds indexing buf with the values in optim.pulse_pos (pulse_cnt is either 5 or 6, and the size of FCBParam::pulse_sign and pulse_pos is PULSE_MAX (6)).

The local variable optim is not initialized in fcb_search(). In get_fcb_param() it's assigned at the end of the function in the /* Minimize */ part, but only if (err < optim->min_err), where optim.min_err = 1 << 30;. err is calculated above that in the /* Compute square of error */ part, by clamping a 64 bit int to 32 bits, so it can easily be larger than 1 << 30. If this happens in all the iterations in get_fcb_param(), then optim is not initialized, and buf is indexed by an uninitialized variable, which caused the crashes.

The fix is to initialize optim in fcb_search(). After we applied the patch to ffmpeg, the compressions did not crash anymore.

Note: this only fixes the crash by ensuring the indices are in the valid range, it doesn't make them correct.

Thanks,
Zsolt


This electronic message may contain proprietary and confidential information of Verint Systems Inc., its affiliates and/or subsidiaries. The information is intended to be for the use of the individual(s) or entity(ies) named above. If you are not the intended recipient (or authorized to receive this e-mail for the intended recipient), you may not use, copy, disclose or distribute to anyone this message or any information contained in this message. If you have received this electronic message in error, please notify us by replying to this e-mail.

[-- Attachment #2: 0001-libavcodec-g723_1enc-fix-crash.patch --]
[-- Type: application/octet-stream, Size: 873 bytes --]

[-- Attachment #3: Type: text/plain, Size: 163 bytes --]

_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

[FFmpeg-devel] Re: [PATCH] libavcodec/g723_1enc: Fix crash

[Michael Niedermayer via ffmpeg-devel]

[-- Attachment #1.1: Type: text/plain, Size: 2117 bytes --]

Hi

On Fri, Oct 03, 2025 at 03:49:36PM +0000, Kovacs, Zsolt via ffmpeg-devel wrote:
> Hi All,
> 
> We had crashes in prod while compressing audio with G.723.1 using ffmpeg as a library. The callstack was:
> 
> fcb_search, line 1027
> ff_encode_encode_cb, line 253
> encode_simple_internal, line 339
> avcodec_send_frame, line 530
> 
> After debugging the issue I found that the crash is caused by FCBParam optim; being uninitialized in fcb_search().
> 
> The context of fcb_search(), line 1027 in libavcodec\g723_1enc.c:
>     /* Reconstruct the excitation */
>     memset(buf, 0, sizeof(int16_t) * SUBFRAME_LEN);
>     for (i = 0; i < pulse_cnt; i++)
>         buf[optim.pulse_pos[i]] = optim.pulse_sign[i];
> 
> The last line is 1027, the crash is caused by out of bounds indexing buf with the values in optim.pulse_pos (pulse_cnt is either 5 or 6, and the size of FCBParam::pulse_sign and pulse_pos is PULSE_MAX (6)).
> 
> The local variable optim is not initialized in fcb_search(). In get_fcb_param() it's assigned at the end of the function in the /* Minimize */ part, but only if (err < optim->min_err), where optim.min_err = 1 << 30;. err is calculated above that in the /* Compute square of error */ part, by clamping a 64 bit int to 32 bits, so it can easily be larger than 1 << 30. If this happens in all the iterations in get_fcb_param(), then optim is not initialized, and buf is indexed by an uninitialized variable, which caused the crashes.
> 
> The fix is to initialize optim in fcb_search(). After we applied the patch to ffmpeg, the compressions did not crash anymore.
> 
> Note: this only fixes the crash by ensuring the indices are in the valid range, it doesn't make them correct.

can you share the testcase that causes this issue ?

or test my pr here: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20658
which may fix this

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The smallest minority on earth is the individual. Those who deny 
individual rights cannot claim to be defenders of minorities. - Ayn Rand

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 163 bytes --]

_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org