[1452] fixed some more corner cases in receiver::pop that have been missed:

- socket family mismatch for the remote endpoint
- cases where the given SA length is too small
corresponding tests were added, too.

diff --git a/src/lib/util/io/socketsession.cc b/src/lib/util/io/socketsession.cc
index 4bf51008a5e5dc535c30e476687588b08b255b85..443e5b64318abf3dee9fa7887871303d1b13cc69 100644 (file)
--- a/src/lib/util/io/socketsession.cc
+++ b/src/lib/util/io/socketsession.cc
@@ -361,18 +361,23 @@ SocketSessionReceiver::pop() {
         const int type = static_cast<int>(ibuffer.readUint32());
         const int protocol = static_cast<int>(ibuffer.readUint32());
         const socklen_t local_end_len = ibuffer.readUint32();
-        if (local_end_len > sizeof(impl_->ss_local_)) {
-            isc_throw(SocketSessionError, "Local SA length too large: " <<
+        const socklen_t endpoint_minlen = (family == AF_INET) ?
+            sizeof(struct sockaddr_in) : sizeof(struct sockaddr_in6);
+        if (local_end_len < endpoint_minlen ||
+            local_end_len > sizeof(impl_->ss_local_)) {
+            isc_throw(SocketSessionError, "Invalid local SA length: " <<
                       local_end_len);
         }
         ibuffer.readData(&impl_->ss_local_, local_end_len);
         const socklen_t remote_end_len = ibuffer.readUint32();
-        if (remote_end_len > sizeof(impl_->ss_remote_)) {
-            isc_throw(SocketSessionError, "Remote SA length too large: " <<
+        if (remote_end_len < endpoint_minlen ||
+            remote_end_len > sizeof(impl_->ss_remote_)) {
+            isc_throw(SocketSessionError, "Invalid remote SA length: " <<
                       remote_end_len);
         }
         ibuffer.readData(&impl_->ss_remote_, remote_end_len);
-        if (family != impl_->sa_local_->sa_family) {
+        if (family != impl_->sa_local_->sa_family ||
+            family != impl_->sa_remote_->sa_family) {
             isc_throw(SocketSessionError, "SA family inconsistent: " <<
                       static_cast<int>(impl_->sa_local_->sa_family) << ", " <<
                       static_cast<int>(impl_->sa_remote_->sa_family) <<

diff --git a/src/lib/util/tests/socketsession_unittest.cc b/src/lib/util/tests/socketsession_unittest.cc
index 154333a0382f64ed0cf2e9b3db55951a6bdcbf6e..23596751e2dfd5e64e03775b1edd020b8ead3867 100644 (file)
--- a/src/lib/util/tests/socketsession_unittest.cc
+++ b/src/lib/util/tests/socketsession_unittest.cc
@@ -25,6 +25,7 @@
 #include <cerrno>
 #include <cstring>
 
+#include <algorithm>
 #include <string>
 #include <utility>
 #include <vector>
@@ -318,9 +319,9 @@ protected:
         obuffer.writeUint32(static_cast<uint32_t>(type));
         obuffer.writeUint32(static_cast<uint32_t>(protocol));
         obuffer.writeUint32(static_cast<uint32_t>(local_len));
-        obuffer.writeData(&local, getSALength(local));
+        obuffer.writeData(&local, min(local_len, getSALength(local)));
         obuffer.writeUint32(static_cast<uint32_t>(remote_len));
-        obuffer.writeData(&remote, getSALength(remote));
+        obuffer.writeData(&remote, min(remote_len, getSALength(remote)));
         obuffer.writeUint32(static_cast<uint32_t>(data_len));
         pushSessionHeader(obuffer.getLength());
         if (send(dummy_forwarder_.fd, obuffer.getData(), obuffer.getLength(),
@@ -751,6 +752,7 @@ TEST_F(ForwardTest, badPop) {
     pushSession(AF_INET, SOCK_DGRAM, IPPROTO_UDP, sai_local.second,
                 *sai_local.first, sai6.second, *sai6.first);
     dummy_forwarder_.reset(-1);
+    EXPECT_THROW(receiver_->pop(), SocketSessionError);
 
     // Pass too big sa length for local
     pushSession(AF_INET, SOCK_DGRAM, IPPROTO_UDP,
@@ -766,6 +768,20 @@ TEST_F(ForwardTest, badPop) {
     dummy_forwarder_.reset(-1);
     EXPECT_THROW(receiver_->pop(), SocketSessionError);
 
+    // Pass too small sa length for local
+    pushSession(AF_INET, SOCK_DGRAM, IPPROTO_UDP,
+                sizeof(struct sockaddr_in) - 1, *sai_local.first,
+                sai_remote.second, *sai_remote.first);
+    dummy_forwarder_.reset(-1);
+    EXPECT_THROW(receiver_->pop(), SocketSessionError);
+
+    // Same for remote
+    pushSession(AF_INET6, SOCK_DGRAM, IPPROTO_UDP,
+                sai6.second, *sai6.first, sizeof(struct sockaddr_in6) - 1,
+                *sai6.first);
+    dummy_forwarder_.reset(-1);
+    EXPECT_THROW(receiver_->pop(), SocketSessionError);
+
     // Data length is too large
     pushSession(AF_INET, SOCK_DGRAM, IPPROTO_UDP, sai_local.second,
                 *sai_local.first, sai_remote.second,