From 1f3560203ce127d108901f52bcfaf7ddc55744f5 Mon Sep 17 00:00:00 2001 From: Won Chung Date: Tue, 7 Jan 2025 10:25:50 -0800 Subject: [PATCH] Address CodeQL security issues Fix comparison of narrow type with wide type in loop condition. Comparison between types of different widths in a loop condition can cause the loop to fail to terminate. --- onnxruntime/core/framework/transpose_helper.cc | 8 ++++---- onnxruntime/core/graph/contrib_ops/contrib_defs.cc | 8 ++++---- onnxruntime/core/graph/graph_utils.cc | 8 ++++---- onnxruntime/core/optimizer/attention_fusion_helper.h | 4 ++-- onnxruntime/core/util/qmath.h | 4 ++-- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/onnxruntime/core/framework/transpose_helper.cc b/onnxruntime/core/framework/transpose_helper.cc index 38f68215a0484..32d15bdf9060b 100644 --- a/onnxruntime/core/framework/transpose_helper.cc +++ b/onnxruntime/core/framework/transpose_helper.cc @@ -27,7 +27,7 @@ typename std::enable_if::value, void>::type SimpleTranspo for (int64_t l = 0; l < num_loops; ++l) { T* output_for_first_writer = output_data; - for (auto wwpl = 0; wwpl < writes_per_writer_per_loop; ++wwpl) { + for (int64_t wwpl = 0; wwpl < writes_per_writer_per_loop; ++wwpl) { T* output_for_current_writer = output_for_first_writer; end = input_data + num_writers; @@ -130,7 +130,7 @@ typename std::enable_if::value, void>::type SimpleTranspo for (int64_t l = 0; l < num_loops; ++l) { const T* input_for_first_reader = input_data; - for (auto rrpl = 0; rrpl < reads_per_reader_per_loop; ++rrpl) { + for (int64_t rrpl = 0; rrpl < reads_per_reader_per_loop; ++rrpl) { const T* input_for_current_reader = input_for_first_reader; end = output_data + num_readers; @@ -210,7 +210,7 @@ void TransposeSingleAxisInwards(gsl::span permutations, const Tens for (int64_t l = 0; l < num_loops; ++l) { const uint8_t* input_for_first_reader = input_data; - for (auto rrpl = 0; rrpl < reads_per_reader_per_loop; ++rrpl) { + for (int64_t rrpl = 0; rrpl < reads_per_reader_per_loop; ++rrpl) { const uint8_t* input_for_current_reader = input_for_first_reader; for (int64_t r = 0; r < num_readers; ++r) { @@ -309,4 +309,4 @@ bool IsTransposeMovingSingleAxis(gsl::span permutations, size_t& f return single_axis_moved; } -} // namespace onnxruntime \ No newline at end of file +} // namespace onnxruntime diff --git a/onnxruntime/core/graph/contrib_ops/contrib_defs.cc b/onnxruntime/core/graph/contrib_ops/contrib_defs.cc index d78fe7111c9be..e45787299f3ad 100644 --- a/onnxruntime/core/graph/contrib_ops/contrib_defs.cc +++ b/onnxruntime/core/graph/contrib_ops/contrib_defs.cc @@ -2890,15 +2890,15 @@ void RegisterContribSchemas() { if (ctx.getNumOutputs() > 1) { auto saved_mean_shape = ctx.getOutputType(1)->mutable_tensor_type()->mutable_shape(); saved_mean_shape->CopyFrom(input_shape); - for (int d = static_cast(axis); d < input_ndim; ++d) - saved_mean_shape->mutable_dim(d)->set_dim_value(1); + for (int64_t d = axis; d < input_ndim; ++d) + saved_mean_shape->mutable_dim(static_cast(d))->set_dim_value(1); } if (ctx.getNumOutputs() > 2) { auto saved_inv_std_dev_shape = ctx.getOutputType(2)->mutable_tensor_type()->mutable_shape(); saved_inv_std_dev_shape->CopyFrom(input_shape); - for (int d = static_cast(axis); d < input_ndim; ++d) - saved_inv_std_dev_shape->mutable_dim(d)->set_dim_value(1); + for (int64_t d = axis; d < input_ndim; ++d) + saved_inv_std_dev_shape->mutable_dim(static_cast(d))->set_dim_value(1); } }) .SetContextDependentFunctionBodyBuilder( diff --git a/onnxruntime/core/graph/graph_utils.cc b/onnxruntime/core/graph/graph_utils.cc index 221bc01f5d15d..640a6c6d4232a 100644 --- a/onnxruntime/core/graph/graph_utils.cc +++ b/onnxruntime/core/graph/graph_utils.cc @@ -869,13 +869,13 @@ bool RemoveNodesWithOneOutputBottomUp(Graph& graph, const Node& start_node) { } // push the parents of current node to the queue. - for (unsigned int i = 0; i < cur_node.InputDefs().size(); ++i) { - const std::string& input_name = GetNodeInputName(cur_node, i); - if (IsInitializer(graph, input_name, true) || IsGraphInput(graph, cur_node.InputDefs()[i])) { + for (size_t i = 0; i < cur_node.InputDefs().size(); ++i) { + const std::string& input_name = GetNodeInputName(cur_node, static_cast(i)); + if (IsInitializer(graph, input_name, true) || IsGraphInput(graph, cur_node.InputDefs()[static_cast(i)])) { // skip initializers and graph inputs continue; } - const Node* parent_node = GetInputNode(cur_node, i); + const Node* parent_node = GetInputNode(cur_node, static_cast(i)); if (nullptr == parent_node) { continue; } diff --git a/onnxruntime/core/optimizer/attention_fusion_helper.h b/onnxruntime/core/optimizer/attention_fusion_helper.h index 935114c40d1a7..aa70b347d7b67 100644 --- a/onnxruntime/core/optimizer/attention_fusion_helper.h +++ b/onnxruntime/core/optimizer/attention_fusion_helper.h @@ -281,8 +281,8 @@ bool ValidateUnidirMask(std::vector mask_data, int64_t w, bool& is_undirectio is_undirectional = true; const T* p = mask_data.data(); - for (int i = 0; i < w; i++) { - for (int j = 0; j < w; j++) { + for (int64_t i = 0; i < w; i++) { + for (int64_t j = 0; j < w; j++) { if (*p != static_cast(1)) { is_one = false; } diff --git a/onnxruntime/core/util/qmath.h b/onnxruntime/core/util/qmath.h index 1b2180da95058..0172902bdf4e2 100644 --- a/onnxruntime/core/util/qmath.h +++ b/onnxruntime/core/util/qmath.h @@ -64,7 +64,7 @@ void GetQuantizationParameter(const float* data, int64_t num_of_elements, float& block_size = onnxruntime::narrow(num_of_elements); } - for (int i = 0; i < num_blocks; i++) { + for (int i = 0; i < narrow(num_blocks); i++) { aggregate[i].min = std::numeric_limits::max(); aggregate[i].max = std::numeric_limits::lowest(); } @@ -79,7 +79,7 @@ void GetQuantizationParameter(const float* data, int64_t num_of_elements, float& float& min = aggregate[0].min; float& max = aggregate[0].max; - for (int i = 1; i < num_blocks; i++) { + for (int i = 1; i < narrow(num_blocks); i++) { min = std::min(min, aggregate[i].min); max = std::max(max, aggregate[i].max); }