From d78ac099d6a69c06f3bb27488e9287250ad56a07 Mon Sep 17 00:00:00 2001 From: Frank Dana Date: Sun, 3 Oct 2021 07:01:56 -0400 Subject: [PATCH] CodeQL fixes, take 2 (#745) * Protect values against integer overflow 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. * Apply these fixes correctly --- src/DummyReader.cpp | 2 +- src/FFmpegReader.cpp | 5 ++--- src/FFmpegWriter.cpp | 26 ++++++++++++-------------- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/DummyReader.cpp b/src/DummyReader.cpp index d8d7d694..6913e3fe 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 = static_cast(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 c5c72a63..714c8dbd 100644 --- a/src/FFmpegReader.cpp +++ b/src/FFmpegReader.cpp @@ -1537,10 +1537,9 @@ void FFmpegReader::ProcessAudioPacket(int64_t requested_frame, int64_t target_fr // Copy audio samples over original samples memcpy(audio_buf, audio_converted->data[0], - static_cast( - audio_converted->nb_samples + static_cast(audio_converted->nb_samples) * av_get_bytes_per_sample(AV_SAMPLE_FMT_S16) - * info.channels) + * info.channels ); // Deallocate resample buffer diff --git a/src/FFmpegWriter.cpp b/src/FFmpegWriter.cpp index 33c6da82..433a2f33 100644 --- a/src/FFmpegWriter.cpp +++ b/src/FFmpegWriter.cpp @@ -1673,9 +1673,9 @@ void FFmpegWriter::write_audio_packets(bool is_final) { // Copy audio samples over original samples memcpy(all_resampled_samples, audio_converted->data[0], - static_cast( - nb_samples * info.channels - * av_get_bytes_per_sample(output_sample_fmt))); + static_cast(nb_samples) + * info.channels + * av_get_bytes_per_sample(output_sample_fmt)); // Remove converted audio av_freep(&(audio_frame->data[0])); @@ -1709,8 +1709,8 @@ void FFmpegWriter::write_audio_packets(bool is_final) { av_get_bytes_per_sample(AV_SAMPLE_FMT_S16) ) ), all_resampled_samples + samples_position, - static_cast( - diff * av_get_bytes_per_sample(output_sample_fmt)) + static_cast(diff) + * av_get_bytes_per_sample(output_sample_fmt) ); // Increment counters @@ -1765,9 +1765,9 @@ void FFmpegWriter::write_audio_packets(bool is_final) { // Copy audio into buffer for frame memcpy(final_samples_planar, samples, - static_cast( - audio_frame->nb_samples * info.channels - * av_get_bytes_per_sample(output_sample_fmt))); + 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, @@ -1793,10 +1793,9 @@ void FFmpegWriter::write_audio_packets(bool is_final) { ); // Copy audio samples over original samples - const auto copy_length = static_cast( - nb_samples + const auto copy_length = static_cast(nb_samples) * av_get_bytes_per_sample(audio_codec_ctx->sample_fmt) - * info.channels); + * info.channels; if (nb_samples > 0) memcpy(samples, frame_final->data[0], copy_length); @@ -1810,10 +1809,9 @@ void FFmpegWriter::write_audio_packets(bool is_final) { } else { // Create a new array - const auto buf_size = static_cast( - 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));