From 7852c7b28c51e725e6fbe864bcdc3ce19def3936 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Rial?= <19420029+rialg@users.noreply.github.com> Date: Wed, 17 Mar 2021 00:12:35 +0100 Subject: [PATCH] Macro wrapper for memcpy (#13770) Initial migration from memcpy to SAFE_MEMCPY Macro and MemBlockBuilder. Fixes #9328 Signed-off-by: grial --- source/common/buffer/buffer_impl.cc | 2 +- source/common/buffer/buffer_impl.h | 6 +-- source/common/chromium_url/BUILD | 5 ++- source/common/chromium_url/url_canon.h | 7 +-- source/common/common/BUILD | 9 ++++ source/common/common/hash.h | 5 ++- source/common/common/mem_block_builder.h | 9 +++- source/common/common/safe_memcpy.h | 38 ++++++++++++++++ source/common/common/utility.cc | 2 +- source/common/grpc/BUILD | 1 + source/common/grpc/common.cc | 5 ++- source/common/http/http2/codec_impl.cc | 4 +- source/common/network/BUILD | 2 + source/common/network/address_impl.cc | 8 ++-- source/common/network/address_impl.h | 1 + source/common/network/cidr_range.cc | 3 +- .../proxy_protocol/proxy_protocol_header.cc | 32 +++++--------- source/extensions/common/wasm/BUILD | 1 + source/extensions/common/wasm/context.cc | 35 ++++++++------- source/extensions/common/wasm/foreign.cc | 6 +-- .../filters/listener/proxy_protocol/BUILD | 1 + .../listener/proxy_protocol/proxy_protocol.cc | 5 ++- .../filters/network/dubbo_proxy/BUILD | 1 + .../network/dubbo_proxy/buffer_helper.cc | 4 +- .../network/dubbo_proxy/buffer_helper.h | 1 + source/extensions/filters/network/kafka/BUILD | 1 + .../filters/network/kafka/serialization.cc | 4 +- .../filters/network/kafka/serialization.h | 16 +++---- .../filters/network/kafka/tagged_fields.h | 2 +- .../filters/network/mongo_proxy/bson_impl.cc | 9 ++-- .../filters/network/thrift_proxy/BUILD | 1 + .../network/thrift_proxy/buffer_helper.cc | 5 ++- .../extensions/filters/udp/dns_filter/BUILD | 1 + .../filters/udp/dns_filter/dns_parser.cc | 5 ++- .../quiche/platform/string_utils.cc | 2 +- .../stat_sinks/common/statsd/statsd.cc | 7 +-- source/extensions/transport_sockets/tls/BUILD | 1 + .../transport_sockets/tls/utility.cc | 5 ++- source/server/BUILD | 2 + source/server/hot_restarting_base.cc | 5 ++- test/common/common/BUILD | 6 +++ test/common/common/safe_memcpy_test.cc | 44 +++++++++++++++++++ test/per_file_coverage.sh | 2 +- tools/code_format/check_format.py | 15 +++++++ 44 files changed, 235 insertions(+), 91 deletions(-) create mode 100644 source/common/common/safe_memcpy.h create mode 100644 test/common/common/safe_memcpy_test.cc diff --git a/source/common/buffer/buffer_impl.cc b/source/common/buffer/buffer_impl.cc index 420dd9cd1d2c..4209ba05674b 100644 --- a/source/common/buffer/buffer_impl.cc +++ b/source/common/buffer/buffer_impl.cc @@ -98,7 +98,7 @@ void OwnedImpl::copyOut(size_t start, uint64_t size, void* data) const { continue; } uint64_t copy_size = std::min(size, data_size - bytes_to_skip); - memcpy(dest, slice.data() + bytes_to_skip, copy_size); + memcpy(dest, slice.data() + bytes_to_skip, copy_size); // NOLINT(safe-memcpy) size -= copy_size; dest += copy_size; // Now that we've started copying, there are no bytes left to skip over. If there diff --git a/source/common/buffer/buffer_impl.h b/source/common/buffer/buffer_impl.h index b217d82195ed..33b9bedb5162 100644 --- a/source/common/buffer/buffer_impl.h +++ b/source/common/buffer/buffer_impl.h @@ -230,7 +230,7 @@ class Slice { uint8_t* dest = base_ + reservable_; reservable_ += copy_size; // NOLINTNEXTLINE(clang-analyzer-core.NullDereference) - memcpy(dest, data, copy_size); + memcpy(dest, data, copy_size); // NOLINT(safe-memcpy) return copy_size; } @@ -260,7 +260,7 @@ class Slice { copy_size = std::min(size, data_); data_ -= copy_size; } - memcpy(base_ + data_, src + size - copy_size, copy_size); + memcpy(base_ + data_, src + size - copy_size, copy_size); // NOLINT(safe-memcpy) return copy_size; } @@ -788,7 +788,7 @@ class OwnedBufferFragmentImpl final : public BufferFragment, public InlineStorag OwnedBufferFragmentImpl(absl::string_view data, const Releasor& releasor) : releasor_(releasor), size_(data.size()) { ASSERT(releasor != nullptr); - memcpy(data_, data.data(), data.size()); + memcpy(data_, data.data(), data.size()); // NOLINT(safe-memcpy) } const Releasor releasor_; diff --git a/source/common/chromium_url/BUILD b/source/common/chromium_url/BUILD index 2d4acb348765..0529a808f139 100644 --- a/source/common/chromium_url/BUILD +++ b/source/common/chromium_url/BUILD @@ -24,5 +24,8 @@ envoy_cc_library( "url_parse.h", "url_parse_internal.h", ], - deps = ["//source/common/common:assert_lib"], + deps = [ + "//source/common/common:assert_lib", + "//source/common/common:mem_block_builder_lib", + ], ) diff --git a/source/common/chromium_url/url_canon.h b/source/common/chromium_url/url_canon.h index 0280de643ac8..a5d0939cb761 100644 --- a/source/common/chromium_url/url_canon.h +++ b/source/common/chromium_url/url_canon.h @@ -13,6 +13,7 @@ #include "common/chromium_url/envoy_shim.h" #include "common/chromium_url/url_parse.h" +#include "common/common/mem_block_builder.h" namespace chromium_url { @@ -145,11 +146,11 @@ template class RawCanonOutputT : public } void Resize(int sz) override { - T* new_buf = new T[sz]; - memcpy(new_buf, this->buffer_, sizeof(T) * (this->cur_len_ < sz ? this->cur_len_ : sz)); + Envoy::MemBlockBuilder new_buf(sz); + new_buf.appendData(absl::Span(this->buffer, std::min(this->cur_len_, sz))); if (this->buffer_ != fixed_buffer_) delete[] this->buffer_; - this->buffer_ = new_buf; + this->buffer_ = new_buf.releasePointer(); this->buffer_len_ = sz; } diff --git a/source/common/common/BUILD b/source/common/common/BUILD index c1751cbf7490..64ecb08d7b21 100644 --- a/source/common/common/BUILD +++ b/source/common/common/BUILD @@ -90,6 +90,11 @@ envoy_cc_library( hdrs = ["enum_to_int.h"], ) +envoy_cc_library( + name = "safe_memcpy_lib", + hdrs = ["safe_memcpy.h"], +) + # fmt_lib is automatically a dependency of all envoy_cc_library definitions. envoy_basic_cc_library( name = "fmt_lib", @@ -107,6 +112,10 @@ envoy_cc_library( srcs = ["hash.cc"], hdrs = ["hash.h"], external_deps = ["xxhash"], + deps = [ + ":macros", + ":safe_memcpy_lib", + ], ) envoy_cc_library( diff --git a/source/common/common/hash.h b/source/common/common/hash.h index c29b9effa89d..13152ef7bb62 100644 --- a/source/common/common/hash.h +++ b/source/common/common/hash.h @@ -2,6 +2,9 @@ #include +#include "common/common/macros.h" +#include "common/common/safe_memcpy.h" + #include "absl/container/flat_hash_map.h" #include "absl/container/flat_hash_set.h" #include "absl/strings/ascii.h" @@ -58,7 +61,7 @@ class MurmurHash { private: static inline uint64_t unalignedLoad(const char* p) { uint64_t result; - memcpy(&result, p, sizeof(result)); + safeMemcpyUnsafeSrc(&result, p); return result; } diff --git a/source/common/common/mem_block_builder.h b/source/common/common/mem_block_builder.h index a43f7e885e22..ce2323c44fa8 100644 --- a/source/common/common/mem_block_builder.h +++ b/source/common/common/mem_block_builder.h @@ -26,7 +26,7 @@ template class MemBlockBuilder { public: // Constructs a MemBlockBuilder allowing for 'capacity' instances of T. explicit MemBlockBuilder(uint64_t capacity) - : data_(std::make_unique(capacity)), write_span_(data_.get(), capacity) {} + : data_(std::make_unique(capacity)), write_span_(data_.get(), capacity){}; MemBlockBuilder() = default; /** @@ -102,6 +102,13 @@ template class MemBlockBuilder { return std::move(data_); } + /** + * Returns the underlying storage as a raw pointer, clearing this. + * + * @return the transferred storage. + */ + T* releasePointer() { return this->release().release(); } + /** * @return the populated data as an absl::Span. */ diff --git a/source/common/common/safe_memcpy.h b/source/common/common/safe_memcpy.h new file mode 100644 index 000000000000..624998e763dd --- /dev/null +++ b/source/common/common/safe_memcpy.h @@ -0,0 +1,38 @@ +#pragma once + +#include + +namespace Envoy { + +/** + * Copies src to dst based on their sizes, which must be the same. + * @param dst the destination + * @param src the source + */ +template inline void safeMemcpy(T1* dst, T2* src) { + static_assert(sizeof(T1) == sizeof(T2)); + memcpy(dst, src, sizeof(T1)); +} + +/** + * Copies src to dst based on the size of dst. + * Sizes are not compared, so ensure the src is of size sizeof(*(dst)) before proceeding to + * call safeMemcpyUnsafeSrc + * @param dst the destination + * @param src the source + */ +template inline void safeMemcpyUnsafeSrc(T1* dst, T2* src) { + memcpy(dst, src, sizeof(T1)); +} +/** + * Copies src to dst based on the size of src. + * Sizes are not compared, so ensure the dst is of size sizeof(*(src)) before proceeding to + * call safeMemcpyUnsafeDst + * @param dst the destination + * @param src the source + */ +template inline void safeMemcpyUnsafeDst(T1* dst, T2* src) { + memcpy(dst, src, sizeof(T2)); +} + +} // namespace Envoy diff --git a/source/common/common/utility.cc b/source/common/common/utility.cc index 47167d41c289..7eb2a08e6bae 100644 --- a/source/common/common/utility.cc +++ b/source/common/common/utility.cc @@ -625,7 +625,7 @@ double WelfordStandardDeviation::computeStandardDeviation() const { InlineString::InlineString(const char* str, size_t size) : size_(size) { RELEASE_ASSERT(size <= 0xffffffff, "size must fit in 32 bits"); - memcpy(data_, str, size); + memcpy(data_, str, size); // NOLINT(safe-memcpy) } void ExceptionUtil::throwEnvoyException(const std::string& message) { diff --git a/source/common/grpc/BUILD b/source/common/grpc/BUILD index 79b0eea9b9f3..c00ce5555e07 100644 --- a/source/common/grpc/BUILD +++ b/source/common/grpc/BUILD @@ -93,6 +93,7 @@ envoy_cc_library( "//source/common/common:enum_to_int", "//source/common/common:hash_lib", "//source/common/common:macros", + "//source/common/common:safe_memcpy_lib", "//source/common/common:utility_lib", "//source/common/grpc:status_lib", "//source/common/http:header_utility_lib", diff --git a/source/common/grpc/common.cc b/source/common/grpc/common.cc index 549be3dbbc72..f673367d179d 100644 --- a/source/common/grpc/common.cc +++ b/source/common/grpc/common.cc @@ -13,6 +13,7 @@ #include "common/common/enum_to_int.h" #include "common/common/fmt.h" #include "common/common/macros.h" +#include "common/common/safe_memcpy.h" #include "common/common/utility.h" #include "common/http/header_utility.h" #include "common/http/headers.h" @@ -136,7 +137,7 @@ Buffer::InstancePtr Common::serializeToGrpcFrame(const Protobuf::Message& messag uint8_t* current = reinterpret_cast(reservation.slice().mem_); *current++ = 0; // flags const uint32_t nsize = htonl(size); - std::memcpy(current, reinterpret_cast(&nsize), sizeof(uint32_t)); + safeMemcpyUnsafeDst(current, &nsize); current += sizeof(uint32_t); Protobuf::io::ArrayOutputStream stream(current, size, -1); Protobuf::io::CodedOutputStream codec_stream(&stream); @@ -288,7 +289,7 @@ void Common::prependGrpcFrameHeader(Buffer::Instance& buffer) { std::array header; header[0] = 0; // flags const uint32_t nsize = htonl(buffer.length()); - std::memcpy(&header[1], reinterpret_cast(&nsize), sizeof(uint32_t)); + safeMemcpyUnsafeDst(&header[1], &nsize); buffer.prepend(absl::string_view(&header[0], 5)); } diff --git a/source/common/http/http2/codec_impl.cc b/source/common/http/http2/codec_impl.cc index da1ef10bed75..19719ef9044b 100644 --- a/source/common/http/http2/codec_impl.cc +++ b/source/common/http/http2/codec_impl.cc @@ -16,6 +16,7 @@ #include "common/common/dump_state_utils.h" #include "common/common/enum_to_int.h" #include "common/common/fmt.h" +#include "common/common/safe_memcpy.h" #include "common/common/scope_tracker.h" #include "common/common/utility.h" #include "common/http/codes.h" @@ -789,8 +790,7 @@ Status ConnectionImpl::onFrameReceived(const nghttp2_frame* frame) { // was the current time when the ping was sent. This can be useful while debugging // to match the ping and ack. uint64_t data; - static_assert(sizeof(data) == sizeof(frame->ping.opaque_data), "Sizes are equal"); - memcpy(&data, frame->ping.opaque_data, sizeof(data)); + safeMemcpy(&data, &(frame->ping.opaque_data)); ENVOY_CONN_LOG(trace, "recv PING ACK {}", connection_, data); onKeepaliveResponse(); diff --git a/source/common/network/BUILD b/source/common/network/BUILD index 39d0f3f95065..5cabdf820f85 100644 --- a/source/common/network/BUILD +++ b/source/common/network/BUILD @@ -17,6 +17,7 @@ envoy_cc_library( "//include/envoy/network:address_interface", "//source/common/api:os_sys_calls_lib", "//source/common/common:assert_lib", + "//source/common/common:safe_memcpy_lib", "//source/common/common:utility_lib", ], ) @@ -40,6 +41,7 @@ envoy_cc_library( ":utility_lib", "//include/envoy/network:address_interface", "//source/common/common:assert_lib", + "//source/common/common:safe_memcpy_lib", "//source/common/common:utility_lib", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", ], diff --git a/source/common/network/address_impl.cc b/source/common/network/address_impl.cc index 38308a491f5b..478299a503ee 100644 --- a/source/common/network/address_impl.cc +++ b/source/common/network/address_impl.cc @@ -9,6 +9,7 @@ #include "common/common/assert.h" #include "common/common/fmt.h" +#include "common/common/safe_memcpy.h" #include "common/common/utility.h" #include "common/network/socket_interface.h" @@ -175,8 +176,7 @@ std::string Ipv4Instance::sockaddrToString(const sockaddr_in& addr) { absl::uint128 Ipv6Instance::Ipv6Helper::address() const { absl::uint128 result{0}; static_assert(sizeof(absl::uint128) == 16, "The size of asbl::uint128 is not 16."); - memcpy(static_cast(&result), static_cast(&address_.sin6_addr.s6_addr), - sizeof(absl::uint128)); + safeMemcpyUnsafeSrc(&result, &address_.sin6_addr.s6_addr[0]); return result; } @@ -281,7 +281,9 @@ PipeInstance::PipeInstance(const std::string& pipe_path, mode_t mode, } pipe_.abstract_namespace_ = true; pipe_.address_length_ = pipe_path.size(); - memcpy(&pipe_.address_.sun_path[0], pipe_path.data(), pipe_path.size()); + // The following statement is safe since pipe_path size was checked at the beginning of this + // function + memcpy(&pipe_.address_.sun_path[0], pipe_path.data(), pipe_path.size()); // NOLINT(safe-memcpy) pipe_.address_.sun_path[0] = '\0'; pipe_.address_.sun_path[pipe_path.size()] = '\0'; friendly_name_ = friendlyNameFromAbstractPath( diff --git a/source/common/network/address_impl.h b/source/common/network/address_impl.h index 11aed2301952..584b5384d140 100644 --- a/source/common/network/address_impl.h +++ b/source/common/network/address_impl.h @@ -227,6 +227,7 @@ class PipeInstance : public InstanceBase { const sockaddr* sockAddr() const override { return reinterpret_cast(&pipe_.address_); } + const sockaddr_un& getSockAddr() const { return pipe_.address_; } socklen_t sockAddrLen() const override { if (pipe_.abstract_namespace_) { return offsetof(struct sockaddr_un, sun_path) + pipe_.address_length_; diff --git a/source/common/network/cidr_range.cc b/source/common/network/cidr_range.cc index 277125e17c4a..2ee249f265c1 100644 --- a/source/common/network/cidr_range.cc +++ b/source/common/network/cidr_range.cc @@ -11,6 +11,7 @@ #include "common/common/assert.h" #include "common/common/fmt.h" +#include "common/common/safe_memcpy.h" #include "common/common/utility.h" #include "common/network/address_impl.h" #include "common/network/utility.h" @@ -183,7 +184,7 @@ InstanceConstSharedPtr CidrRange::truncateIpAddressAndLength(InstanceConstShared absl::uint128 ip6_htonl = Utility::Ip6htonl(ip6); static_assert(sizeof(absl::uint128) == 16, "The size of asbl::uint128 is not 16."); - memcpy(&sa6.sin6_addr.s6_addr, &ip6_htonl, sizeof(absl::uint128)); + safeMemcpy(&sa6.sin6_addr.s6_addr, &ip6_htonl); return std::make_shared(sa6); } } diff --git a/source/extensions/common/proxy_protocol/proxy_protocol_header.cc b/source/extensions/common/proxy_protocol/proxy_protocol_header.cc index 454bcfd420d0..123bff19f2b7 100644 --- a/source/extensions/common/proxy_protocol/proxy_protocol_header.cc +++ b/source/extensions/common/proxy_protocol/proxy_protocol_header.cc @@ -66,39 +66,31 @@ void generateV2Header(const std::string& src_addr, const std::string& dst_addr, case Network::Address::IpVersion::v4: { addr_length[1] = PROXY_PROTO_V2_ADDR_LEN_INET; out.add(addr_length, 2); - - uint8_t addrs[8]; - const auto net_src_addr = + const uint32_t net_src_addr = Network::Address::Ipv4Instance(src_addr, src_port).ip()->ipv4()->address(); - const auto net_dst_addr = + const uint32_t net_dst_addr = Network::Address::Ipv4Instance(dst_addr, dst_port).ip()->ipv4()->address(); - memcpy(addrs, &net_src_addr, 4); - memcpy(&addrs[4], &net_dst_addr, 4); - out.add(addrs, 8); + out.add(&net_src_addr, 4); + out.add(&net_dst_addr, 4); break; } case Network::Address::IpVersion::v6: { addr_length[1] = PROXY_PROTO_V2_ADDR_LEN_INET6; out.add(addr_length, 2); - - uint8_t addrs[32]; - const auto net_src_addr = + const absl::uint128 net_src_addr = Network::Address::Ipv6Instance(src_addr, src_port).ip()->ipv6()->address(); - const auto net_dst_addr = + const absl::uint128 net_dst_addr = Network::Address::Ipv6Instance(dst_addr, dst_port).ip()->ipv6()->address(); - memcpy(addrs, &net_src_addr, 16); - memcpy(&addrs[16], &net_dst_addr, 16); - out.add(addrs, 32); + out.add(&net_src_addr, 16); + out.add(&net_dst_addr, 16); break; } } - uint8_t ports[4]; - const auto net_src_port = htons(static_cast(src_port)); - const auto net_dst_port = htons(static_cast(dst_port)); - memcpy(ports, &net_src_port, 2); - memcpy(&ports[2], &net_dst_port, 2); - out.add(ports, 4); + const uint16_t net_src_port = htons(static_cast(src_port)); + const uint16_t net_dst_port = htons(static_cast(dst_port)); + out.add(&net_src_port, 2); + out.add(&net_dst_port, 2); } void generateV2Header(const Network::Address::Ip& source_address, diff --git a/source/extensions/common/wasm/BUILD b/source/extensions/common/wasm/BUILD index 7d005bf051a2..4c872edec4bf 100644 --- a/source/extensions/common/wasm/BUILD +++ b/source/extensions/common/wasm/BUILD @@ -83,6 +83,7 @@ envoy_cc_library( "//include/envoy/server:lifecycle_notifier_interface", "//source/common/buffer:buffer_lib", "//source/common/common:enum_to_int", + "//source/common/common:safe_memcpy_lib", "//source/common/config:remote_data_fetcher_lib", "//source/common/http:message_lib", "//source/common/http:utility_lib", diff --git a/source/extensions/common/wasm/context.cc b/source/extensions/common/wasm/context.cc index 8cc8f7ea57fb..d5e1d097fc7d 100644 --- a/source/extensions/common/wasm/context.cc +++ b/source/extensions/common/wasm/context.cc @@ -20,6 +20,7 @@ #include "common/common/empty_string.h" #include "common/common/enum_to_int.h" #include "common/common/logger.h" +#include "common/common/safe_memcpy.h" #include "common/http/header_map_impl.h" #include "common/http/message_impl.h" #include "common/http/utility.h" @@ -200,15 +201,16 @@ void Context::onResolveDns(uint32_t token, Envoy::Network::DnsResolver::Resoluti auto buffer = std::unique_ptr(new char[s]); char* b = buffer.get(); uint32_t n = response.size(); - memcpy(b, &n, sizeof(uint32_t)); + safeMemcpyUnsafeDst(b, &n); b += sizeof(uint32_t); for (auto& e : response) { uint32_t ttl = e.ttl_.count(); - memcpy(b, &ttl, sizeof(uint32_t)); + safeMemcpyUnsafeDst(b, &ttl); b += sizeof(uint32_t); }; for (auto& e : response) { - memcpy(b, e.address_->asStringView().data(), e.address_->asStringView().size()); + memcpy(b, e.address_->asStringView().data(), // NOLINT(safe-memcpy) + e.address_->asStringView().size()); b += e.address_->asStringView().size(); *b++ = 0; }; @@ -269,45 +271,46 @@ void Context::onStatsUpdate(Envoy::Stats::MetricSnapshot& snapshot) { auto buffer = std::unique_ptr(new char[counter_block_size + gauge_block_size]); char* b = buffer.get(); - memcpy(b, &counter_block_size, sizeof(uint32_t)); + safeMemcpyUnsafeDst(b, &counter_block_size); b += sizeof(uint32_t); - memcpy(b, &counter_type, sizeof(uint32_t)); + safeMemcpyUnsafeDst(b, &counter_type); b += sizeof(uint32_t); - memcpy(b, &num_counters, sizeof(uint32_t)); + safeMemcpyUnsafeDst(b, &num_counters); b += sizeof(uint32_t); for (const auto& counter : snapshot.counters()) { if (counter.counter_.get().used()) { n = counter.counter_.get().name().size(); - memcpy(b, &n, sizeof(uint32_t)); + safeMemcpyUnsafeDst(b, &n); b += sizeof(uint32_t); - memcpy(b, counter.counter_.get().name().data(), counter.counter_.get().name().size()); + memcpy(b, counter.counter_.get().name().data(), // NOLINT(safe-memcpy) + counter.counter_.get().name().size()); b = align(b + counter.counter_.get().name().size()); v = counter.counter_.get().value(); - memcpy(b, &v, sizeof(uint64_t)); + safeMemcpyUnsafeDst(b, &v); b += sizeof(uint64_t); v = counter.delta_; - memcpy(b, &v, sizeof(uint64_t)); + safeMemcpyUnsafeDst(b, &v); b += sizeof(uint64_t); } } - memcpy(b, &gauge_block_size, sizeof(uint32_t)); + safeMemcpyUnsafeDst(b, &gauge_block_size); b += sizeof(uint32_t); - memcpy(b, &gauge_type, sizeof(uint32_t)); + safeMemcpyUnsafeDst(b, &gauge_type); b += sizeof(uint32_t); - memcpy(b, &num_gauges, sizeof(uint32_t)); + safeMemcpyUnsafeDst(b, &num_gauges); b += sizeof(uint32_t); for (const auto& gauge : snapshot.gauges()) { if (gauge.get().used()) { n = gauge.get().name().size(); - memcpy(b, &n, sizeof(uint32_t)); + safeMemcpyUnsafeDst(b, &n); b += sizeof(uint32_t); - memcpy(b, gauge.get().name().data(), gauge.get().name().size()); + memcpy(b, gauge.get().name().data(), gauge.get().name().size()); // NOLINT(safe-memcpy) b = align(b + gauge.get().name().size()); v = gauge.get().value(); - memcpy(b, &v, sizeof(uint64_t)); + safeMemcpyUnsafeDst(b, &v); b += sizeof(uint64_t); } } diff --git a/source/extensions/common/wasm/foreign.cc b/source/extensions/common/wasm/foreign.cc index 7bfe6a02af19..2d59dcc43e1f 100644 --- a/source/extensions/common/wasm/foreign.cc +++ b/source/extensions/common/wasm/foreign.cc @@ -37,7 +37,7 @@ RegisterForeignFunction registerCompressForeignFunction( return WasmResult::SerializationFailure; } auto result = alloc_result(dest_len); - memcpy(result, b.get(), dest_len); + memcpy(result, b.get(), dest_len); // NOLINT(safe-memcpy) return WasmResult::Ok; }); @@ -53,7 +53,7 @@ RegisterForeignFunction registerUncompressForeignFunction( arguments.size()); if (r == Z_OK) { auto result = alloc_result(dest_len); - memcpy(result, b.get(), dest_len); + memcpy(result, b.get(), dest_len); // NOLINT(safe-memcpy) return WasmResult::Ok; } if (r != Z_BUF_ERROR) { @@ -186,7 +186,7 @@ class EvaluateExpressionFactory : public ExpressionFactory { return serialize_status; } auto output = alloc_result(result.size()); - memcpy(output, result.data(), result.size()); + memcpy(output, result.data(), result.size()); // NOLINT(safe-memcpy) return WasmResult::Ok; }; return f; diff --git a/source/extensions/filters/listener/proxy_protocol/BUILD b/source/extensions/filters/listener/proxy_protocol/BUILD index 96840051a6f1..66c21a7b2768 100644 --- a/source/extensions/filters/listener/proxy_protocol/BUILD +++ b/source/extensions/filters/listener/proxy_protocol/BUILD @@ -27,6 +27,7 @@ envoy_cc_library( "//source/common/common:assert_lib", "//source/common/common:empty_string", "//source/common/common:minimal_logger_lib", + "//source/common/common:safe_memcpy_lib", "//source/common/common:utility_lib", "//source/common/network:address_lib", "//source/common/network:utility_lib", diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index 3250236255b2..320337fcba16 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -17,6 +17,7 @@ #include "common/common/assert.h" #include "common/common/empty_string.h" #include "common/common/fmt.h" +#include "common/common/safe_memcpy.h" #include "common/common/utility.h" #include "common/network/address_impl.h" #include "common/network/utility.h" @@ -221,11 +222,11 @@ bool Filter::parseV2Header(char* buf) { memset(&la6, 0, sizeof(la6)); ra6.sin6_family = AF_INET6; ra6.sin6_port = v6->src_port; - memcpy(ra6.sin6_addr.s6_addr, v6->src_addr, sizeof(ra6.sin6_addr.s6_addr)); + safeMemcpy(&(ra6.sin6_addr.s6_addr), &(v6->src_addr)); la6.sin6_family = AF_INET6; la6.sin6_port = v6->dst_port; - memcpy(la6.sin6_addr.s6_addr, v6->dst_addr, sizeof(la6.sin6_addr.s6_addr)); + safeMemcpy(&(la6.sin6_addr.s6_addr), &(v6->dst_addr)); proxy_protocol_header_.emplace(WireHeader{ hdr_addr_len - PROXY_PROTO_V2_ADDR_LEN_INET6, Network::Address::IpVersion::v6, diff --git a/source/extensions/filters/network/dubbo_proxy/BUILD b/source/extensions/filters/network/dubbo_proxy/BUILD index 4ccdd989436f..db011190e626 100644 --- a/source/extensions/filters/network/dubbo_proxy/BUILD +++ b/source/extensions/filters/network/dubbo_proxy/BUILD @@ -17,6 +17,7 @@ envoy_cc_library( "//source/common/buffer:buffer_lib", "//source/common/common:assert_lib", "//source/common/common:byte_order_lib", + "//source/common/common:safe_memcpy_lib", ], ) diff --git a/source/extensions/filters/network/dubbo_proxy/buffer_helper.cc b/source/extensions/filters/network/dubbo_proxy/buffer_helper.cc index ee7c762b565a..934a8ffb780a 100644 --- a/source/extensions/filters/network/dubbo_proxy/buffer_helper.cc +++ b/source/extensions/filters/network/dubbo_proxy/buffer_helper.cc @@ -11,7 +11,7 @@ double BufferHelper::peekDouble(Buffer::Instance& buffer, uint64_t offset) { } double i; uint64_t j = buffer.peekBEInt(offset); - std::memcpy(&i, &j, 8); + safeMemcpy(&i, &j); return i; } @@ -21,7 +21,7 @@ float BufferHelper::peekFloat(Buffer::Instance& buffer, uint64_t offset) { } float i; uint32_t j = buffer.peekBEInt(offset); - std::memcpy(&i, &j, 4); + safeMemcpy(&i, &j); return i; } } // namespace DubboProxy diff --git a/source/extensions/filters/network/dubbo_proxy/buffer_helper.h b/source/extensions/filters/network/dubbo_proxy/buffer_helper.h index d3020c39c0f9..cdd25c939f4b 100644 --- a/source/extensions/filters/network/dubbo_proxy/buffer_helper.h +++ b/source/extensions/filters/network/dubbo_proxy/buffer_helper.h @@ -4,6 +4,7 @@ #include "envoy/common/exception.h" #include "common/common/assert.h" +#include "common/common/safe_memcpy.h" namespace Envoy { namespace Extensions { diff --git a/source/extensions/filters/network/kafka/BUILD b/source/extensions/filters/network/kafka/BUILD index 6ad3666863c9..01c31e63cc9f 100644 --- a/source/extensions/filters/network/kafka/BUILD +++ b/source/extensions/filters/network/kafka/BUILD @@ -231,6 +231,7 @@ envoy_cc_library( ":kafka_types_lib", "//include/envoy/buffer:buffer_interface", "//source/common/common:byte_order_lib", + "//source/common/common:safe_memcpy_lib", ], ) diff --git a/source/extensions/filters/network/kafka/serialization.cc b/source/extensions/filters/network/kafka/serialization.cc index cc818bc7f292..1f1d2643f3a8 100644 --- a/source/extensions/filters/network/kafka/serialization.cc +++ b/source/extensions/filters/network/kafka/serialization.cc @@ -73,7 +73,7 @@ uint32_t feedBytesIntoBuffers(absl::string_view& data, DeserializerType& length_ const uint32_t data_consumed = std::min(required, data.size()); const uint32_t written = data_buffer.size() - required; if (data_consumed > 0) { - memcpy(data_buffer.data() + written, data.data(), data_consumed); + memcpy(data_buffer.data() + written, data.data(), data_consumed); // NOLINT(safe-memcpy) required -= data_consumed; data = {data.data() + data_consumed, data.size() - data_consumed}; } @@ -167,7 +167,7 @@ feedCompactBytesIntoBuffers(absl::string_view& data, VarUInt32Deserializer& leng const uint32_t data_consumed = std::min(required, data.size()); const uint32_t written = data_buffer.size() - required; if (data_consumed > 0) { - memcpy(data_buffer.data() + written, data.data(), data_consumed); + memcpy(data_buffer.data() + written, data.data(), data_consumed); // NOLINT(safe-memcpy) required -= data_consumed; data = {data.data() + data_consumed, data.size() - data_consumed}; } diff --git a/source/extensions/filters/network/kafka/serialization.h b/source/extensions/filters/network/kafka/serialization.h index 65a2b18e78e7..6e2fe85f145b 100644 --- a/source/extensions/filters/network/kafka/serialization.h +++ b/source/extensions/filters/network/kafka/serialization.h @@ -11,6 +11,7 @@ #include "common/common/byte_order.h" #include "common/common/fmt.h" +#include "common/common/safe_memcpy.h" #include "common/common/utility.h" #include "extensions/filters/network/kafka/kafka_types.h" @@ -69,7 +70,7 @@ template class IntDeserializer : public Deserializer { public: uint32_t feed(absl::string_view& data) override { const uint32_t available = std::min(sizeof(buf_) - written_, data.size()); - memcpy(buf_ + written_, data.data(), available); + memcpy(buf_ + written_, data.data(), available); // NOLINT(safe-memcpy) written_ += available; if (written_ == sizeof(buf_)) { @@ -95,8 +96,7 @@ template class IntDeserializer : public Deserializer { class Int8Deserializer : public IntDeserializer { public: int8_t get() const override { - int8_t result; - memcpy(&result, buf_, sizeof(result)); + int8_t result = buf_[0]; return result; } }; @@ -108,7 +108,7 @@ class Int16Deserializer : public IntDeserializer { public: int16_t get() const override { int16_t result; - memcpy(&result, buf_, sizeof(result)); + safeMemcpyUnsafeSrc(&result, buf_); return be16toh(result); } }; @@ -120,7 +120,7 @@ class Int32Deserializer : public IntDeserializer { public: int32_t get() const override { int32_t result; - memcpy(&result, buf_, sizeof(result)); + safeMemcpyUnsafeSrc(&result, buf_); return be32toh(result); } }; @@ -132,7 +132,7 @@ class UInt32Deserializer : public IntDeserializer { public: uint32_t get() const override { uint32_t result; - memcpy(&result, buf_, sizeof(result)); + safeMemcpyUnsafeSrc(&result, buf_); return be32toh(result); } }; @@ -144,7 +144,7 @@ class Int64Deserializer : public IntDeserializer { public: int64_t get() const override { int64_t result; - memcpy(&result, buf_, sizeof(result)); + safeMemcpyUnsafeSrc(&result, buf_); return be64toh(result); } }; @@ -189,7 +189,7 @@ class VarUInt32Deserializer : public Deserializer { // Read next byte from input. uint8_t el; - memcpy(&el, data.data(), sizeof(uint8_t)); + safeMemcpy(&el, data.data()); data = {data.data() + 1, data.size() - 1}; processed++; diff --git a/source/extensions/filters/network/kafka/tagged_fields.h b/source/extensions/filters/network/kafka/tagged_fields.h index 7e60952c03ce..0678d9053f32 100644 --- a/source/extensions/filters/network/kafka/tagged_fields.h +++ b/source/extensions/filters/network/kafka/tagged_fields.h @@ -70,7 +70,7 @@ class TaggedFieldDeserializer : public Deserializer { const uint32_t data_consumed = std::min(required_, data.size()); const uint32_t written = data_buffer_.size() - required_; if (data_consumed > 0) { - memcpy(data_buffer_.data() + written, data.data(), data_consumed); + memcpy(data_buffer_.data() + written, data.data(), data_consumed); // NOLINT(safe-memcpy) required_ -= data_consumed; data = {data.data() + data_consumed, data.size() - data_consumed}; } diff --git a/source/extensions/filters/network/mongo_proxy/bson_impl.cc b/source/extensions/filters/network/mongo_proxy/bson_impl.cc index 4d7638762646..de0b9170b22b 100644 --- a/source/extensions/filters/network/mongo_proxy/bson_impl.cc +++ b/source/extensions/filters/network/mongo_proxy/bson_impl.cc @@ -22,8 +22,7 @@ int32_t BufferHelper::peekInt32(Buffer::Instance& data) { } int32_t val; - void* mem = data.linearize(sizeof(int32_t)); - std::memcpy(reinterpret_cast(&val), mem, sizeof(int32_t)); + val = data.peekLEInt(); return le32toh(val); } @@ -44,7 +43,7 @@ void BufferHelper::removeBytes(Buffer::Instance& data, uint8_t* out, size_t out_ } void* mem = data.linearize(out_len); - std::memcpy(out, mem, out_len); + std::memcpy(out, mem, out_len); // NOLINT(safe-memcpy) data.drain(out_len); } @@ -88,9 +87,7 @@ int64_t BufferHelper::removeInt64(Buffer::Instance& data) { } int64_t val; - void* mem = data.linearize(sizeof(int64_t)); - std::memcpy(reinterpret_cast(&val), mem, sizeof(int64_t)); - data.drain(sizeof(int64_t)); + val = data.drainLEInt(); return le64toh(val); } diff --git a/source/extensions/filters/network/thrift_proxy/BUILD b/source/extensions/filters/network/thrift_proxy/BUILD index 3d098813e3c3..47439e4c53ea 100644 --- a/source/extensions/filters/network/thrift_proxy/BUILD +++ b/source/extensions/filters/network/thrift_proxy/BUILD @@ -28,6 +28,7 @@ envoy_cc_library( "//source/common/buffer:buffer_lib", "//source/common/common:assert_lib", "//source/common/common:byte_order_lib", + "//source/common/common:safe_memcpy_lib", ], ) diff --git a/source/extensions/filters/network/thrift_proxy/buffer_helper.cc b/source/extensions/filters/network/thrift_proxy/buffer_helper.cc index 8267b0313441..48167e024bed 100644 --- a/source/extensions/filters/network/thrift_proxy/buffer_helper.cc +++ b/source/extensions/filters/network/thrift_proxy/buffer_helper.cc @@ -1,6 +1,7 @@ #include "extensions/filters/network/thrift_proxy/buffer_helper.h" #include "common/common/byte_order.h" +#include "common/common/safe_memcpy.h" namespace Envoy { namespace Extensions { @@ -21,7 +22,7 @@ double BufferHelper::drainBEDouble(Buffer::Instance& buffer) { // 4. Implementation of last resort is to manually copy from i to d via unsigned char*. uint64_t i = buffer.drainBEInt(); double d; - std::memcpy(&d, &i, 8); + safeMemcpy(&d, &i); return d; } @@ -121,7 +122,7 @@ void BufferHelper::writeBEDouble(Buffer::Instance& buffer, double value) { // See drainDouble for implementation details. uint64_t i; - std::memcpy(&i, &value, 8); + safeMemcpy(&i, &value); buffer.writeBEInt(i); } diff --git a/source/extensions/filters/udp/dns_filter/BUILD b/source/extensions/filters/udp/dns_filter/BUILD index ee87d26ef8de..210d68496d0d 100644 --- a/source/extensions/filters/udp/dns_filter/BUILD +++ b/source/extensions/filters/udp/dns_filter/BUILD @@ -35,6 +35,7 @@ envoy_cc_library( "//source/common/buffer:buffer_lib", "//source/common/common:empty_string", "//source/common/common:matchers_lib", + "//source/common/common:safe_memcpy_lib", "//source/common/config:config_provider_lib", "//source/common/config:datasource_lib", "//source/common/network:address_lib", diff --git a/source/extensions/filters/udp/dns_filter/dns_parser.cc b/source/extensions/filters/udp/dns_filter/dns_parser.cc index b779120c942b..26899ea8ad13 100644 --- a/source/extensions/filters/udp/dns_filter/dns_parser.cc +++ b/source/extensions/filters/udp/dns_filter/dns_parser.cc @@ -2,6 +2,7 @@ #include "envoy/network/address.h" +#include "common/common/safe_memcpy.h" #include "common/network/address_impl.h" #include "common/network/utility.h" @@ -171,7 +172,7 @@ bool DnsMessageParser::parseDnsObject(DnsQueryContextPtr& context, state = DnsQueryParseState::Flags; break; case DnsQueryParseState::Flags: - ::memcpy(static_cast(&context->header_.flags), &data, field_size); + safeMemcpyUnsafeDst(static_cast(&context->header_.flags), &data); state = DnsQueryParseState::Questions; break; case DnsQueryParseState::Questions: @@ -743,7 +744,7 @@ void DnsMessageParser::buildResponseBuffer(DnsQueryContextPtr& query_context, buffer.writeBEInt(query_context->response_header_.id); uint16_t flags; - ::memcpy(&flags, static_cast(&query_context->response_header_.flags), sizeof(uint16_t)); + safeMemcpyUnsafeSrc(&flags, static_cast(&query_context->response_header_.flags)); buffer.writeBEInt(flags); buffer.writeBEInt(query_context->response_header_.questions); diff --git a/source/extensions/quic_listeners/quiche/platform/string_utils.cc b/source/extensions/quic_listeners/quiche/platform/string_utils.cc index 24ef55bfe94a..e72130705720 100644 --- a/source/extensions/quic_listeners/quiche/platform/string_utils.cc +++ b/source/extensions/quic_listeners/quiche/platform/string_utils.cc @@ -87,7 +87,7 @@ bool HexDecodeToUInt32(absl::string_view data, uint32_t* out) { RELEASE_ASSERT(byte_string.size() == 4u, "padded data is not 4 byte long."); uint32_t bytes; - memcpy(&bytes, byte_string.data(), byte_string.length()); + memcpy(&bytes, byte_string.data(), byte_string.length()); // NOLINT(safe-memcpy) *out = ntohl(bytes); return true; } diff --git a/source/extensions/stat_sinks/common/statsd/statsd.cc b/source/extensions/stat_sinks/common/statsd/statsd.cc index a1ad60cf1607..ae37b96f0548 100644 --- a/source/extensions/stat_sinks/common/statsd/statsd.cc +++ b/source/extensions/stat_sinks/common/statsd/statsd.cc @@ -210,10 +210,11 @@ void TcpStatsdSink::TlsSink::commonFlush(const std::string& name, uint64_t value // This written this way for maximum perf since with a large number of stats and at a high flush // rate this can become expensive. const char* snapped_current = current_slice_mem_; - memcpy(current_slice_mem_, parent_.getPrefix().c_str(), parent_.getPrefix().size()); - current_slice_mem_ += parent_.getPrefix().size(); + const std::string prefix = parent_.getPrefix(); + memcpy(current_slice_mem_, prefix.data(), prefix.size()); // NOLINT(safe-memcpy) + current_slice_mem_ += prefix.size(); *current_slice_mem_++ = '.'; - memcpy(current_slice_mem_, name.c_str(), name.size()); + memcpy(current_slice_mem_, name.data(), name.size()); // NOLINT(safe-memcpy) current_slice_mem_ += name.size(); *current_slice_mem_++ = ':'; current_slice_mem_ += StringUtil::itoa(current_slice_mem_, 30, value); diff --git a/source/extensions/transport_sockets/tls/BUILD b/source/extensions/transport_sockets/tls/BUILD index 1622e9101655..b05b7770a131 100644 --- a/source/extensions/transport_sockets/tls/BUILD +++ b/source/extensions/transport_sockets/tls/BUILD @@ -199,6 +199,7 @@ envoy_cc_library( deps = [ "//source/common/common:assert_lib", "//source/common/common:empty_string", + "//source/common/common:safe_memcpy_lib", "//source/common/common:utility_lib", "//source/common/network:address_lib", ], diff --git a/source/extensions/transport_sockets/tls/utility.cc b/source/extensions/transport_sockets/tls/utility.cc index 1f99484634f0..628477af3120 100644 --- a/source/extensions/transport_sockets/tls/utility.cc +++ b/source/extensions/transport_sockets/tls/utility.cc @@ -2,6 +2,7 @@ #include "common/common/assert.h" #include "common/common/empty_string.h" +#include "common/common/safe_memcpy.h" #include "common/network/address_impl.h" #include "common/protobuf/utility.h" @@ -150,14 +151,14 @@ std::string Utility::generalNameAsString(const GENERAL_NAME* general_name) { sockaddr_in sin; sin.sin_port = 0; sin.sin_family = AF_INET; - memcpy(&sin.sin_addr, general_name->d.ip->data, sizeof(sin.sin_addr)); + safeMemcpyUnsafeSrc(&sin.sin_addr, general_name->d.ip->data); Network::Address::Ipv4Instance addr(&sin); san = addr.ip()->addressAsString(); } else if (general_name->d.ip->length == 16) { sockaddr_in6 sin6; sin6.sin6_port = 0; sin6.sin6_family = AF_INET6; - memcpy(&sin6.sin6_addr, general_name->d.ip->data, sizeof(sin6.sin6_addr)); + safeMemcpyUnsafeSrc(&sin6.sin6_addr, general_name->d.ip->data); Network::Address::Ipv6Instance addr(sin6); san = addr.ip()->addressAsString(); } diff --git a/source/server/BUILD b/source/server/BUILD index db77e5f3fb0c..911137aeea51 100644 --- a/source/server/BUILD +++ b/source/server/BUILD @@ -184,6 +184,8 @@ envoy_cc_library( "//include/envoy/stats:stats_interface", "//source/common/api:os_sys_calls_lib", "//source/common/common:assert_lib", + "//source/common/common:mem_block_builder_lib", + "//source/common/common:safe_memcpy_lib", "//source/common/common:utility_lib", "//source/common/network:utility_lib", "//source/common/stats:utility_lib", diff --git a/source/server/hot_restarting_base.cc b/source/server/hot_restarting_base.cc index 917f50bd8f53..ea8424f1778e 100644 --- a/source/server/hot_restarting_base.cc +++ b/source/server/hot_restarting_base.cc @@ -1,6 +1,8 @@ #include "server/hot_restarting_base.h" #include "common/api/os_sys_calls_impl.h" +#include "common/common/mem_block_builder.h" +#include "common/common/safe_memcpy.h" #include "common/common/utility.h" #include "common/network/address_impl.h" #include "common/stats/utility.h" @@ -38,8 +40,9 @@ sockaddr_un HotRestartingBase::createDomainSocketAddress(uint64_t id, const std: initDomainSocketAddress(&address); Network::Address::PipeInstance addr(fmt::format(socket_path + "_{}_{}", role, base_id_ + id), socket_mode, nullptr); - memcpy(&address, addr.sockAddr(), addr.sockAddrLen()); + safeMemcpy(&address, &(addr.getSockAddr())); fchmod(my_domain_socket_, socket_mode); + return address; } diff --git a/test/common/common/BUILD b/test/common/common/BUILD index 42f276872236..1ae4ee700141 100644 --- a/test/common/common/BUILD +++ b/test/common/common/BUILD @@ -86,6 +86,12 @@ envoy_cc_test( deps = ["//source/common/common:mem_block_builder_lib"], ) +envoy_cc_test( + name = "safe_memcpy_test", + srcs = ["safe_memcpy_test.cc"], + deps = ["//source/common/common:safe_memcpy_lib"], +) + envoy_cc_test( name = "phantom_test", srcs = ["phantom_test.cc"], diff --git a/test/common/common/safe_memcpy_test.cc b/test/common/common/safe_memcpy_test.cc new file mode 100644 index 000000000000..bbe8010205ec --- /dev/null +++ b/test/common/common/safe_memcpy_test.cc @@ -0,0 +1,44 @@ +#include + +#include "common/common/safe_memcpy.h" +#include "common/grpc/common.h" +#include "common/http/headers.h" +#include "common/http/message_impl.h" +#include "common/http/utility.h" + +#include "test/test_common/utility.h" + +#include "gtest/gtest.h" + +using testing::ElementsAre; + +namespace Envoy { + +TEST(SafeMemcpyTest, CopyUint8) { + const uint8_t src[] = {0, 1, 1, 2, 3, 5, 8, 13}; + uint8_t dst[8]; + safeMemcpy(&dst, &src); + EXPECT_THAT(dst, ElementsAre(0, 1, 1, 2, 3, 5, 8, 13)); +} + +TEST(SafeMemcpyUnsafeSrcTest, CopyUint8Pointer) { + uint8_t* src = new uint8_t[8]; + for (int i = 0; i < 8; ++i) { + src[i] = i; + } + uint8_t dst[8]; + safeMemcpyUnsafeSrc(&dst, src); + EXPECT_THAT(dst, ElementsAre(0, 1, 2, 3, 4, 5, 6, 7)); + delete[] src; +} + +TEST(SafeMemcpyUnsafeDstTest, PrependGrpcFrameHeader) { + const uint8_t src[] = {1, 2, 3, 4}; + uint8_t* dst = new uint8_t[4]; + memset(dst, 0, 4); + safeMemcpyUnsafeDst(dst, &src); + EXPECT_THAT(std::vector(dst, dst + sizeof(src)), ElementsAre(1, 2, 3, 4)); + delete[] dst; +} + +} // namespace Envoy diff --git a/test/per_file_coverage.sh b/test/per_file_coverage.sh index 41b3491adeb0..727026dc49e0 100755 --- a/test/per_file_coverage.sh +++ b/test/per_file_coverage.sh @@ -43,7 +43,7 @@ declare -a KNOWN_LOW_COVERAGE=( "source/extensions/filters/network/common/redis:96.3" "source/extensions/filters/network/dubbo_proxy:96.1" "source/extensions/filters/network/dubbo_proxy/router:95.1" -"source/extensions/filters/network/mongo_proxy:94.1" +"source/extensions/filters/network/mongo_proxy:94.0" "source/extensions/filters/network/sni_cluster:90.3" "source/extensions/filters/network/sni_dynamic_forward_proxy:90.9" "source/extensions/health_checkers:95.9" diff --git a/tools/code_format/check_format.py b/tools/code_format/check_format.py index 16544f6a09f1..83454b73ffd0 100755 --- a/tools/code_format/check_format.py +++ b/tools/code_format/check_format.py @@ -107,6 +107,10 @@ # Only one C++ file should instantiate grpc_init GRPC_INIT_ALLOWLIST = ("./source/common/grpc/google_grpc_context.cc") +# Files that should not raise an error for using memcpy +MEMCPY_WHITELIST = ("./source/common/common/mem_block_builder.h", + "./source/common/common/safe_memcpy.h") + # These files should not throw exceptions. Add HTTP/1 when exceptions removed. EXCEPTION_DENYLIST = ("./source/common/http/http2/codec_impl.h", "./source/common/http/http2/codec_impl.cc") @@ -426,6 +430,9 @@ def allowlistedForStdRegex(self, file_path): def allowlistedForGrpcInit(self, file_path): return file_path in GRPC_INIT_ALLOWLIST + def whitelistedForMemcpy(self, file_path): + return file_path in MEMCPY_WHITELIST + def allowlistedForUnpackTo(self, file_path): return file_path.startswith("./test") or file_path in [ "./source/common/protobuf/utility.cc", "./source/common/protobuf/utility.h" @@ -827,6 +834,14 @@ def checkSourceLine(self, line, file_path, reportError): reportError("Don't call grpc_init() or grpc_shutdown() directly, instantiate " + "Grpc::GoogleGrpcContext. See #8282") + if not self.whitelistedForMemcpy(file_path) and \ + not ("test/" in file_path) and \ + ("memcpy(" in line) and \ + not ("NOLINT(safe-memcpy)" in line): + reportError( + "Don't call memcpy() directly; use safeMemcpy, safeMemcpyUnsafeSrc, safeMemcpyUnsafeDst or MemBlockBuilder instead." + ) + if self.denylistedForExceptions(file_path): # Skpping cases where 'throw' is a substring of a symbol like in "foothrowBar". if "throw" in line.split():