Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing security vulnerabilities reported by MSVC security checks #6487

Merged
merged 2 commits into from
May 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,14 @@ jobs:
name: Running Regressions Tests
when: always
command: |
ctest --timeout 60 -T test --no-compress-output --output-on-failure -R tests.regressions
ctest \
--timeout 60 \
-T test \
--no-compress-output \
--output-on-failure \
-R tests.regressions \
--exclude-regex \
"tests.regressions.modules.threading_base.thread_stacksize_current"
- run:
<<: *convert_xml
- run:
Expand Down
5 changes: 4 additions & 1 deletion .github/workflows/codeql.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ jobs:
queries: security-and-quality

- name: Install CMake
uses: ssrobins/install-cmake@v1
uses: jwlawson/actions-setup-cmake@v2.0
with:
cmake-version: '3.22.x'

- name: Install Ninja
uses: seanmiddleditch/gha-setup-ninja@master
Expand Down Expand Up @@ -80,6 +82,7 @@ jobs:
# filter out all files from downloaded dependencies
patterns: |
-**/_deps/**
-**/concurrentqueue.hpp
input: sarif-results/cpp.sarif
output: sarif-results/cpp.sarif

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ namespace hpx {
copy_from(rhs);
}

partitioned_vector(partitioned_vector&& rhs)
partitioned_vector(partitioned_vector&& rhs) noexcept
: base_type(HPX_MOVE(rhs))
, size_(rhs.size_)
, partition_size_(rhs.partition_size_)
Expand Down Expand Up @@ -479,7 +479,7 @@ namespace hpx {
return *this;
}

partitioned_vector& operator=(partitioned_vector&& rhs)
partitioned_vector& operator=(partitioned_vector&& rhs) noexcept
{
if (this != &rhs)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,14 @@ namespace hpx { namespace server {
return *this;
}

partition_unordered_map(partition_unordered_map&& rhs)
partition_unordered_map(partition_unordered_map&& rhs) noexcept
: base_type(HPX_MOVE(rhs))
, partition_unordered_map_(HPX_MOVE(rhs.partition_unordered_map_))
{
}

partition_unordered_map& operator=(partition_unordered_map&& rhs)
partition_unordered_map& operator=(
partition_unordered_map&& rhs) noexcept
{
if (this != &rhs)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ namespace hpx { namespace server {
template <typename Archive>
void serialize(Archive& ar, unsigned)
{
ar& partition_& locality_id_;
ar & partition_ & locality_id_;
}
};

Expand All @@ -101,7 +101,7 @@ namespace hpx { namespace server {
template <typename Archive>
void serialize(Archive& ar, unsigned)
{
ar& partitions_;
ar & partitions_;
}
};
}} // namespace hpx::server
Expand Down Expand Up @@ -593,9 +593,9 @@ namespace hpx {
copy_from(rhs);
}

unordered_map(unordered_map&& rhs)
: base_type(HPX_MOVE(rhs))
, hash_base_type(HPX_MOVE(rhs))
unordered_map(unordered_map&& rhs) noexcept
: base_type(HPX_MOVE(static_cast<base_type&&>(rhs)))
, hash_base_type(HPX_MOVE(static_cast<hash_base_type&&>(rhs)))
, partitions_(HPX_MOVE(rhs.partitions_))
{
}
Expand All @@ -606,7 +606,7 @@ namespace hpx {
copy_from(rhs);
return *this;
}
unordered_map& operator=(unordered_map&& rhs)
unordered_map& operator=(unordered_map&& rhs) noexcept
{
if (this != &rhs)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,8 +239,16 @@ namespace hpx { namespace iostreams {
// Create the next buffer, returns the previous buffer
buffer next = this->detail::buffer::init_locked();

// Unlock the mutex before we cleanup.
// 26110: Caller failing to hold lock 'l'
#if defined(HPX_MSVC)
#pragma warning(push)
#pragma warning(disable : 26110)
#endif
// Unlock the mutex before we clean up.
l.unlock();
#if defined(HPX_MSVC)
#pragma warning(pop)
#endif

// Perform the write operation, then destroy the old buffer and
// stream.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ namespace hpx { namespace iostreams { namespace detail {
{
}

buffer(buffer&& rhs)
buffer(buffer&& rhs) noexcept
: data_(HPX_MOVE(rhs.data_))
, mtx_(HPX_MOVE(rhs.mtx_))
{
Expand All @@ -54,7 +54,7 @@ namespace hpx { namespace iostreams { namespace detail {
return *this;
}

buffer& operator=(buffer&& rhs)
buffer& operator=(buffer&& rhs) noexcept
{
if (this != &rhs)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2016-2022 Hartmut Kaiser
// Copyright (c) 2016-2024 Hartmut Kaiser
//
// SPDX-License-Identifier: BSL-1.0
// Distributed under the Boost Software License, Version 1.0. (See accompanying
Expand Down Expand Up @@ -49,7 +49,7 @@ namespace hpx::plugins::parcel {
get_counter_type average_time_between_parcels;
get_counter_values_creator_type
time_between_parcels_histogram_creator;
std::int64_t min_boundary, max_boundary, num_buckets;
std::int64_t min_boundary = 0, max_boundary = 0, num_buckets = 0;
};

using map_type = std::unordered_map<std::string, counter_functions,
Expand All @@ -60,10 +60,11 @@ namespace hpx::plugins::parcel {
void register_action(std::string const& name);

void register_action(std::string const& name,
get_counter_type num_parcels, get_counter_type num_messages,
get_counter_type time_between_parcels,
get_counter_type average_time_between_parcels,
get_counter_values_creator_type
get_counter_type const& num_parcels,
get_counter_type const& num_messages,
get_counter_type const& time_between_parcels,
get_counter_type const& average_time_between_parcels,
get_counter_values_creator_type const&
time_between_parcels_histogram_creator);

get_counter_type get_parcels_counter(std::string const& name) const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ namespace hpx::plugins::parcel::detail {
{
}

message_buffer& operator=(message_buffer&& rhs)
message_buffer& operator=(message_buffer&& rhs) noexcept
{
if (&rhs != this)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2016-2022 Hartmut Kaiser
// Copyright (c) 2016-2024 Hartmut Kaiser
//
// SPDX-License-Identifier: BSL-1.0
// Distributed under the Boost Software License, Version 1.0. (See accompanying
Expand Down Expand Up @@ -30,10 +30,12 @@ namespace hpx::plugins::parcel {

///////////////////////////////////////////////////////////////////////////
void coalescing_counter_registry::register_action(std::string const& name,
get_counter_type num_parcels, get_counter_type num_messages,
get_counter_type num_parcels_per_message,
get_counter_type average_time_between_parcels,
get_counter_values_creator_type time_between_parcels_histogram_creator)
get_counter_type const& num_parcels,
get_counter_type const& num_messages,
get_counter_type const& num_parcels_per_message,
get_counter_type const& average_time_between_parcels,
get_counter_values_creator_type const&
time_between_parcels_histogram_creator)
{
if (name.empty())
{
Expand All @@ -56,29 +58,28 @@ namespace hpx::plugins::parcel {
else
{
// replace the existing functions
(*it).second.num_parcels = num_parcels;
(*it).second.num_messages = num_messages;
(*it).second.num_parcels_per_message = num_parcels_per_message;
(*it).second.average_time_between_parcels =
it->second.num_parcels = num_parcels;
it->second.num_messages = num_messages;
it->second.num_parcels_per_message = num_parcels_per_message;
it->second.average_time_between_parcels =
average_time_between_parcels;
(*it).second.time_between_parcels_histogram_creator =
it->second.time_between_parcels_histogram_creator =
time_between_parcels_histogram_creator;

if ((*it).second.min_boundary != (*it).second.max_boundary)
if (it->second.min_boundary != it->second.max_boundary)
{
// instantiate actual histogram collection
coalescing_counter_registry::get_counter_values_type result;
time_between_parcels_histogram_creator(
(*it).second.min_boundary, (*it).second.max_boundary,
(*it).second.num_buckets, result);
time_between_parcels_histogram_creator(it->second.min_boundary,
it->second.max_boundary, it->second.num_buckets, result);
}

// silence warnings
(void) (*it).second.num_parcels;
(void) (*it).second.num_messages;
(void) (*it).second.num_parcels_per_message;
(void) (*it).second.average_time_between_parcels;
(void) (*it).second.time_between_parcels_histogram_creator;
(void) it->second.num_parcels;
(void) it->second.num_messages;
(void) it->second.num_parcels_per_message;
(void) it->second.average_time_between_parcels;
(void) it->second.time_between_parcels_histogram_creator;
}
}

Expand Down Expand Up @@ -114,9 +115,8 @@ namespace hpx::plugins::parcel {
HPX_THROW_EXCEPTION(hpx::error::bad_parameter,
"coalescing_counter_registry::get_num_parcels_counter",
"unknown action type");
return get_counter_type();
}
return (*it).second.num_parcels;
return it->second.num_parcels;
}

coalescing_counter_registry::get_counter_type
Expand All @@ -132,9 +132,8 @@ namespace hpx::plugins::parcel {
HPX_THROW_EXCEPTION(hpx::error::bad_parameter,
"coalescing_counter_registry::get_num_messages_counter",
"unknown action type");
return get_counter_type();
}
return (*it).second.num_messages;
return it->second.num_messages;
}

coalescing_counter_registry::get_counter_type
Expand All @@ -150,9 +149,8 @@ namespace hpx::plugins::parcel {
HPX_THROW_EXCEPTION(hpx::error::bad_parameter,
"coalescing_counter_registry::get_num_messages_counter",
"unknown action type");
return get_counter_type();
}
return (*it).second.num_parcels_per_message;
return it->second.num_parcels_per_message;
}

coalescing_counter_registry::get_counter_type
Expand All @@ -169,9 +167,8 @@ namespace hpx::plugins::parcel {
"coalescing_counter_registry::"
"get_average_time_between_parcels_counter",
"unknown action type");
return get_counter_type();
}
return (*it).second.average_time_between_parcels;
return it->second.average_time_between_parcels;
}

coalescing_counter_registry::get_counter_values_type
Expand All @@ -189,20 +186,19 @@ namespace hpx::plugins::parcel {
"coalescing_counter_registry::"
"get_time_between_parcels_histogram_counter",
"unknown action type");
return &coalescing_counter_registry::empty_histogram;
}

if ((*it).second.time_between_parcels_histogram_creator.empty())
if (it->second.time_between_parcels_histogram_creator.empty())
{
// no parcel of this type has been sent yet
(*it).second.min_boundary = min_boundary;
(*it).second.max_boundary = max_boundary;
(*it).second.num_buckets = num_buckets;
it->second.min_boundary = min_boundary;
it->second.max_boundary = max_boundary;
it->second.num_buckets = num_buckets;
return coalescing_counter_registry::get_counter_values_type();
}

coalescing_counter_registry::get_counter_values_type result;
(*it).second.time_between_parcels_histogram_creator(
it->second.time_between_parcels_histogram_creator(
min_boundary, max_boundary, num_buckets, result);
return result;
}
Expand Down Expand Up @@ -281,11 +277,11 @@ namespace hpx::plugins::parcel {
for (map_type::const_iterator it = map_.begin(); it != end;
++it)
{
if (!std::regex_match((*it).first, rx))
if (!std::regex_match(it->first, rx))
continue;

performance_counters::counter_path_elements cp = p;
cp.parameters_ = (*it).first;
cp.parameters_ = it->first;
if (!additional_parameters.empty())
cp.parameters_ += additional_parameters;

Expand All @@ -304,7 +300,7 @@ namespace hpx::plugins::parcel {
for (map_type::const_iterator it = map_.begin(); it != end;
++it)
{
types += " " + (*it).first + "\n";
types += " " + it->first + "\n";
}
}

Expand Down Expand Up @@ -348,10 +344,10 @@ namespace hpx::plugins::parcel {
// compose a list of known action types
std::string types;
map_type::const_iterator end = map_.end();
for (map_type::const_iterator it = map_.begin(); it != end;
++it)
for (map_type::const_iterator it_ct = map_.begin();
it_ct != end; ++it_ct)
{
types += " " + (*it).first + "\n";
types += " " + it_ct->first + "\n";
}

l.unlock();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,16 @@ namespace hpx::plugins::parcel {
std::swap(buff, buffer_);

++num_messages_;

// 26110: Caller failing to hold lock 'l'
#if defined(HPX_MSVC)
#pragma warning(push)
#pragma warning(disable : 26110)
#endif
l.unlock();
#if defined(HPX_MSVC)
#pragma warning(pop)
#endif

HPX_ASSERT(nullptr != pp_);
buff(pp_); // 'invoke' the buffer
Expand Down
Loading
Loading