envoyproxy · jmarantz · Mar 16, 2021 · Oct 25, 2020 · Oct 27, 2020 · Oct 27, 2020
diff --git a/source/common/buffer/buffer_impl.cc b/source/common/buffer/buffer_impl.cc
@@ -125,6 +125,7 @@ void OwnedImpl::commit(RawSlice* iovecs, uint64_t num_iovecs) {
 void OwnedImpl::copyOut(size_t start, uint64_t size, void* data) const {
   uint64_t bytes_to_skip = start;
   uint8_t* dest = static_cast<uint8_t*>(data);
+  MemBlockBuilder<uint8_t> mem_builder(size);
   for (const auto& slice : slices_) {
     if (size == 0) {
       break;
@@ -137,14 +138,14 @@ 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);
+    mem_builder.appendData(slice->data() + bytes_to_skip);
     size -= copy_size;
-    dest += copy_size;
     // Now that we've started copying, there are no bytes left to skip over. If there
     // is any more data to be copied, the next iteration can start copying from the very
     // beginning of the next slice.
     bytes_to_skip = 0;
   }
+  dest = mem_builder.release().get();
   ASSERT(size == 0);
 }
 

diff --git a/source/common/buffer/buffer_impl.h b/source/common/buffer/buffer_impl.h
@@ -8,6 +8,7 @@
 #include "envoy/buffer/buffer.h"
 
 #include "common/common/assert.h"
+#include "common/common/mem_block_builder.h"
 #include "common/common/non_copyable.h"
 #include "common/common/utility.h"
 #include "common/event/libevent.h"
@@ -153,7 +154,9 @@ class Slice : public SliceData {
     uint8_t* dest = base_ + reservable_;
     reservable_ += copy_size;
     // NOLINTNEXTLINE(clang-analyzer-core.NullDereference)
-    memcpy(dest, data, copy_size);
+    MemBlockBuilder<uint8_t> mem_builder(copy_size);
+    mem_builder.appendData(data);
+    dest = mem_builder.release().get();
     return copy_size;
   }
 
@@ -276,7 +279,9 @@ class OwnedSlice final : public Slice, public InlineStorage {
   static SlicePtr create(const void* data, uint64_t size) {
     uint64_t slice_capacity = sliceSize(size);
     std::unique_ptr<OwnedSlice> slice(new (slice_capacity) OwnedSlice(slice_capacity));
-    memcpy(slice->base_, data, size);
+    MemBlockBuilder<uint8_t> mem_builder(size);
+    mem_builder.appendData(data);
+    slice->base_ = mem_builder.release().get();
     slice->reservable_ = size;
     return slice;
   }
@@ -653,7 +658,9 @@ 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());
+    MemBlockBuilder<uint8_t> mem_builder(data.size());
+    mem_builder.appendData(data.data());
+    data_ = mem_builder.release().get();
   }
 
   const Releasor releasor_;

diff --git a/source/common/common/macros.h b/source/common/common/macros.h
@@ -41,6 +41,15 @@ namespace Envoy {
     return *objectptr;                                                                             \
   } while (0)
 
+/**
+ * @brief Assert memory bounds to avoid copy errors.
+ */
+#define SAFE_MEMCPY(dst, src)                                                                      \
+  do {                                                                                             \
+    static_assert(sizeof(*(src)) == sizeof(*(dst)));                                               \
+    memmove(dst, src, sizeof(*(src)));                                                             \
+  } while (0)
+
 /**
  * Have a generic fall-through for different versions of C++
  */

diff --git a/source/common/network/address_impl.h b/source/common/network/address_impl.h
@@ -233,6 +233,7 @@ class PipeInstance : public InstanceBase {
     }
     return sizeof(pipe_.address_);
   }
+  void getSockAddr(sockaddr_un& address) { address = pipe_.address_; }
 
 private:
   struct PipeHelper : public Pipe {

@@ -196,24 +196,28 @@ void Filter::parseV2Header(char* buf) {
                        std::make_shared<Network::Address::Ipv4Instance>(&la4)});
         return;
       } else if (((proto_family & 0xf0) >> 4) == PROXY_PROTO_V2_AF_INET6) {
-        PACKED_STRUCT(struct pp_ipv6_addr {
-          uint8_t src_addr[16];
-          uint8_t dst_addr[16];
-          uint16_t src_port;
-          uint16_t dst_port;
-        });
-        pp_ipv6_addr* v6;
-        v6 = reinterpret_cast<pp_ipv6_addr*>(&buf[PROXY_PROTO_V2_HEADER_LEN]);
+        std::size_t offset = 0; // Offset to iterate over buf
         sockaddr_in6 ra6, la6;
         memset(&ra6, 0, sizeof(ra6));
         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));
-
         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));
+
+        for (; offset < ARRAY_SIZE(ra6.sin6_addr.s6_addr); ++offset)
+          ra6.sin6_addr.s6_addr[offset] = buf[PROXY_PROTO_V2_HEADER_LEN + offset];
+
+        for (; offset < ARRAY_SIZE(la6.sin6_addr.s6_addr); ++offset)
+          la6.sin6_addr.s6_addr[offset] = buf[PROXY_PROTO_V2_HEADER_LEN + offset];
+
+        PACKED_STRUCT(struct ports {
+          uint16_t src_port;
+          uint16_t dst_port;
+        });
+        ports* port_pair;
+        port_pair = reinterpret_cast<ports*>(&buf[PROXY_PROTO_V2_HEADER_LEN + offset]);
+
+        ra6.sin6_port = port_pair->src_port;
+        la6.sin6_port = port_pair->dst_port;
 
         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/mongo_proxy/BUILD b/source/extensions/filters/network/mongo_proxy/BUILD
@@ -34,6 +34,7 @@ envoy_cc_library(
         "//source/common/common:assert_lib",
         "//source/common/common:byte_order_lib",
         "//source/common/common:hex_lib",
+        "//source/common/common:mem_block_builder_lib",
         "//source/common/common:minimal_logger_lib",
         "//source/common/common:utility_lib",
     ],

@@ -8,6 +8,7 @@
 #include "common/common/byte_order.h"
 #include "common/common/fmt.h"
 #include "common/common/hex.h"
+#include "common/common/mem_block_builder.h"
 #include "common/common/utility.h"
 
 namespace Envoy {
@@ -22,8 +23,7 @@ int32_t BufferHelper::peekInt32(Buffer::Instance& data) {
   }
 
   int32_t val;
-  void* mem = data.linearize(sizeof(int32_t));
-  std::memcpy(reinterpret_cast<void*>(&val), mem, sizeof(int32_t));
+  SAFE_MEMCPY(&val, static_cast<int32_t*>(data.linearize(sizeof(int32_t))));
   return le32toh(val);
 }
 
@@ -43,8 +43,10 @@ void BufferHelper::removeBytes(Buffer::Instance& data, uint8_t* out, size_t out_
     throw EnvoyException("invalid buffer size");
   }
 
-  void* mem = data.linearize(out_len);
-  std::memcpy(out, mem, out_len);
+  MemBlockBuilder<uint8_t> mem_builder(out_len);
+  mem_builder.appendData(
+      absl::Span<uint8_t>(static_cast<uint8_t*>(data.linearize(out_len)), out_len));
+  out = mem_builder.release().get();
   data.drain(out_len);
 }
 
@@ -88,8 +90,7 @@ int64_t BufferHelper::removeInt64(Buffer::Instance& data) {
   }
 
   int64_t val;
-  void* mem = data.linearize(sizeof(int64_t));
-  std::memcpy(reinterpret_cast<void*>(&val), mem, sizeof(int64_t));
+  SAFE_MEMCPY(&val, static_cast<int64_t*>(data.linearize(sizeof(int64_t))));
   data.drain(sizeof(int64_t));
   return le64toh(val);
 }

@@ -171,7 +171,7 @@ bool DnsMessageParser::parseDnsObject(DnsQueryContextPtr& context,
       state = DnsQueryParseState::Flags;
       break;
     case DnsQueryParseState::Flags:
-      ::memcpy(static_cast<void*>(&header_.flags), &data, field_size);
+      SAFE_MEMCPY(&(header_.flags), &data);
       state = DnsQueryParseState::Questions;
       break;
     case DnsQueryParseState::Questions:
@@ -741,7 +741,7 @@ void DnsMessageParser::buildResponseBuffer(DnsQueryContextPtr& query_context,
   buffer.writeBEInt<uint16_t>(response_header_.id);
 
   uint16_t flags;
-  ::memcpy(&flags, static_cast<void*>(&response_header_.flags), sizeof(uint16_t));
+  SAFE_MEMCPY(&flags, &(response_header_.flags));
   buffer.writeBEInt<uint16_t>(flags);
 
   buffer.writeBEInt<uint16_t>(response_header_.questions);

diff --git a/source/server/BUILD b/source/server/BUILD
@@ -153,6 +153,7 @@ 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: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
@@ -1,6 +1,7 @@
 #include "server/hot_restarting_base.h"
 
 #include "common/api/os_sys_calls_impl.h"
+#include "common/common/mem_block_builder.h"
 #include "common/common/utility.h"
 #include "common/network/address_impl.h"
 #include "common/stats/utility.h"
@@ -36,8 +37,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());
+  addr.getSockAddr(address);
   fchmod(my_domain_socket_, socket_mode);
+
   return address;
 }