network: fix misaligned access in setsockopt().

Fixes envoyproxy#7968. Signed-off-by: Piotr Sikora <piotrsikora@google.com>
PiotrSikora · Aug 20, 2019 · ff615aa · ff615aa
1 parent f90e1b0
commit ff615aa
Show file tree

Hide file tree

Showing 7 changed files with 12 additions and 10 deletions.
diff --git a/include/envoy/network/listen_socket.h b/include/envoy/network/listen_socket.h
@@ -111,7 +111,7 @@ class Socket {
      */
     struct Details {
       SocketOptionName name_;
-      std::string value_; ///< Binary string representation of an option's value.
+      std::vector<uint8_t> value_; ///< Binary string representation of an option's value.
 
       bool operator==(const Details& other) const {
         return name_ == other.name_ && value_ == other.value_;

diff --git a/source/common/network/socket_option_impl.cc b/source/common/network/socket_option_impl.cc
@@ -47,14 +47,14 @@ bool SocketOptionImpl::isSupported() const { return optname_.has_value(); }
 
 Api::SysCallIntResult SocketOptionImpl::setSocketOption(Socket& socket,
                                                         const Network::SocketOptionName& optname,
-                                                        const absl::string_view value) {
+                                                        const std::vector<uint8_t>& value) {
   if (!optname.has_value()) {
     return {-1, ENOTSUP};
   }
 
   auto& os_syscalls = Api::OsSysCallsSingleton::get();
   return os_syscalls.setsockopt(socket.ioHandle().fd(), optname.level(), optname.option(),
-                                value.data(), value.size());
+                                static_cast<const void*>(value.data()), value.size());
 }
 
 } // namespace Network

diff --git a/source/common/network/socket_option_impl.h b/source/common/network/socket_option_impl.h
@@ -103,7 +103,7 @@ class SocketOptionImpl : public Socket::Option, Logger::Loggable<Logger::Id::con
 
   SocketOptionImpl(envoy::api::v2::core::SocketOption::SocketState in_state,
                    Network::SocketOptionName optname, absl::string_view value)
-      : in_state_(in_state), optname_(optname), value_(value) {}
+      : in_state_(in_state), optname_(optname), value_(value.begin(), value.end()) {}
 
   // Socket::Option
   bool setOption(Socket& socket,
@@ -128,12 +128,12 @@ class SocketOptionImpl : public Socket::Option, Logger::Loggable<Logger::Id::con
    */
   static Api::SysCallIntResult setSocketOption(Socket& socket,
                                                const Network::SocketOptionName& optname,
-                                               absl::string_view value);
+                                               const std::vector<uint8_t>& value);
 
 private:
   const envoy::api::v2::core::SocketOption::SocketState in_state_;
   const Network::SocketOptionName optname_;
-  const std::string value_;
+  const std::vector<uint8_t> value_;
 };
 
 } // namespace Network

diff --git a/test/common/network/socket_option_impl_test.cc b/test/common/network/socket_option_impl_test.cc
@@ -7,7 +7,7 @@ namespace {
 class SocketOptionImplTest : public SocketOptionTest {};
 
 TEST_F(SocketOptionImplTest, BadFd) {
-  absl::string_view zero("\0\0\0\0", 4);
+  std::vector<uint8_t> zero{'\0', '\0', '\0', '\0'};
   Api::SysCallIntResult result = SocketOptionImpl::setSocketOption(socket_, {}, zero);
   EXPECT_EQ(-1, result.rc_);
   EXPECT_EQ(ENOTSUP, result.errno_);

diff --git a/test/common/network/socket_option_test.h b/test/common/network/socket_option_test.h
@@ -73,7 +73,7 @@ class SocketOptionTest : public testing::Test {
 
     Socket::Option::Details expected_info;
     expected_info.name_ = name;
-    expected_info.value_ = std::string(value_as_bstr);
+    expected_info.value_ = std::vector<uint8_t>(value_as_bstr.begin(), value_as_bstr.end());
 
     return expected_info;
   }

diff --git a/test/extensions/filters/http/original_src/original_src_test.cc b/test/extensions/filters/http/original_src/original_src_test.cc
@@ -170,7 +170,8 @@ TEST_F(OriginalSrcHttpTest, FilterAddsMarkOption) {
   ASSERT_TRUE(mark_option.has_value());
   uint32_t value = 1234;
   absl::string_view value_as_bstr(reinterpret_cast<const char*>(&value), sizeof(value));
-  EXPECT_EQ(value_as_bstr, mark_option->value_);
+  std::vector<uint8_t> value_as_vector(value_as_bstr.begin(), value_as_bstr.end());
+  EXPECT_EQ(value_as_vector, mark_option->value_);
 }
 
 TEST_F(OriginalSrcHttpTest, Mark0NotAdded) {

diff --git a/test/extensions/filters/listener/original_src/original_src_test.cc b/test/extensions/filters/listener/original_src/original_src_test.cc
@@ -156,7 +156,8 @@ TEST_F(OriginalSrcTest, filterAddsMarkOption) {
   ASSERT_TRUE(mark_option.has_value());
   uint32_t value = 1234;
   absl::string_view value_as_bstr(reinterpret_cast<const char*>(&value), sizeof(value));
-  EXPECT_EQ(value_as_bstr, mark_option->value_);
+  std::vector<uint8_t> value_as_vector(value_as_bstr.begin(), value_as_bstr.end());
+  EXPECT_EQ(value_as_vector, mark_option->value_);
 }
 
 TEST_F(OriginalSrcTest, Mark0NotAdded) {