From 3d18347e71f36b3daf58c88756ddbf41b385706f Mon Sep 17 00:00:00 2001 From: Frank Dana Date: Mon, 27 Sep 2021 20:36:56 -0400 Subject: [PATCH] Protect values against integer overflow (#743) When the code multiplies integer values in an rvalue context before it's stored in a larger type, the on-the-fly math is stored as int. The value can overflow before it reaches the wider memory space. To prevent this, we explicitly cast the result of the arithmetic to the destination type. Issues flagged by GitHub CodeQL. --- src/DummyReader.cpp | 2 +- src/FFmpegReader.cpp | 8 +++++++- src/FFmpegWriter.cpp | 34 +++++++++++++++++++++++----------- src/Frame.cpp | 6 ++++-- 4 files changed, 35 insertions(+), 15 deletions(-) diff --git a/src/DummyReader.cpp b/src/DummyReader.cpp index e6c5fce1..d8d7d694 100644 --- a/src/DummyReader.cpp +++ b/src/DummyReader.cpp @@ -38,7 +38,7 @@ void DummyReader::init(Fraction fps, int width, int height, int sample_rate, int // Set key info settings info.has_audio = false; info.has_video = true; - info.file_size = width * height * sizeof(int); + info.file_size = static_cast(width * height * sizeof(int)); info.vcodec = "raw"; info.fps = fps; info.width = width; diff --git a/src/FFmpegReader.cpp b/src/FFmpegReader.cpp index c1eaa748..c5c72a63 100644 --- a/src/FFmpegReader.cpp +++ b/src/FFmpegReader.cpp @@ -1535,7 +1535,13 @@ void FFmpegReader::ProcessAudioPacket(int64_t requested_frame, int64_t target_fr audio_frame->nb_samples); // number of input samples to convert // Copy audio samples over original samples - memcpy(audio_buf, audio_converted->data[0], audio_converted->nb_samples * av_get_bytes_per_sample(AV_SAMPLE_FMT_S16) * info.channels); + memcpy(audio_buf, + audio_converted->data[0], + static_cast( + audio_converted->nb_samples + * av_get_bytes_per_sample(AV_SAMPLE_FMT_S16) + * info.channels) + ); // Deallocate resample buffer SWR_CLOSE(avr); diff --git a/src/FFmpegWriter.cpp b/src/FFmpegWriter.cpp index 8707756c..33c6da82 100644 --- a/src/FFmpegWriter.cpp +++ b/src/FFmpegWriter.cpp @@ -1672,7 +1672,10 @@ void FFmpegWriter::write_audio_packets(bool is_final) { ); // Copy audio samples over original samples - memcpy(all_resampled_samples, audio_converted->data[0], nb_samples * info.channels * av_get_bytes_per_sample(output_sample_fmt)); + memcpy(all_resampled_samples, audio_converted->data[0], + static_cast( + nb_samples * info.channels + * av_get_bytes_per_sample(output_sample_fmt))); // Remove converted audio av_freep(&(audio_frame->data[0])); @@ -1706,7 +1709,8 @@ void FFmpegWriter::write_audio_packets(bool is_final) { av_get_bytes_per_sample(AV_SAMPLE_FMT_S16) ) ), all_resampled_samples + samples_position, - diff * av_get_bytes_per_sample(output_sample_fmt) + static_cast( + diff * av_get_bytes_per_sample(output_sample_fmt)) ); // Increment counters @@ -1760,7 +1764,10 @@ void FFmpegWriter::write_audio_packets(bool is_final) { ); // Copy audio into buffer for frame - memcpy(final_samples_planar, samples, audio_frame->nb_samples * info.channels * av_get_bytes_per_sample(output_sample_fmt)); + memcpy(final_samples_planar, samples, + static_cast( + audio_frame->nb_samples * info.channels + * av_get_bytes_per_sample(output_sample_fmt))); // Fill input frame with sample data avcodec_fill_audio_frame(audio_frame, info.channels, output_sample_fmt, @@ -1786,10 +1793,13 @@ void FFmpegWriter::write_audio_packets(bool is_final) { ); // Copy audio samples over original samples - if (nb_samples > 0) { - memcpy(samples, frame_final->data[0], - nb_samples * av_get_bytes_per_sample(audio_codec_ctx->sample_fmt) * info.channels); - } + const auto copy_length = static_cast( + nb_samples + * av_get_bytes_per_sample(audio_codec_ctx->sample_fmt) + * info.channels); + + if (nb_samples > 0) + memcpy(samples, frame_final->data[0], copy_length); // deallocate AVFrame av_freep(&(audio_frame->data[0])); @@ -1800,11 +1810,13 @@ void FFmpegWriter::write_audio_packets(bool is_final) { } else { // Create a new array - final_samples = (int16_t *) av_malloc( - sizeof(int16_t) * audio_input_position + const auto buf_size = static_cast( + audio_input_position * (av_get_bytes_per_sample(audio_codec_ctx->sample_fmt) / - av_get_bytes_per_sample(AV_SAMPLE_FMT_S16) ) - ); + av_get_bytes_per_sample(AV_SAMPLE_FMT_S16)) + ); + final_samples = reinterpret_cast( + av_malloc(sizeof(int16_t) * buf_size)); // Copy audio into buffer for frame memcpy(final_samples, samples, diff --git a/src/Frame.cpp b/src/Frame.cpp index aed38777..28c04eef 100644 --- a/src/Frame.cpp +++ b/src/Frame.cpp @@ -438,8 +438,10 @@ juce::AudioSampleBuffer *Frame::GetAudioSampleBuffer() int64_t Frame::GetBytes() { int64_t total_bytes = 0; - if (image) - total_bytes += (width * height * sizeof(char) * 4); + if (image) { + total_bytes += static_cast( + width * height * sizeof(char) * 4); + } if (audio) { // approximate audio size (sample rate / 24 fps) total_bytes += (sample_rate / 24.0) * sizeof(float);