msg/msg_types: fix the entity_addr_t's decoder

the daemon is vulnerable to malicious client, which is able to send
large elen, and corrupt the stack, etc.

* throw at seeing corrupted entity_addr_t where its elen exceeds
  the length of sockaddr
* handle the exception thrown when decoding entity_addr_t in messenger
  layer.
* if a malicious client manages to send a corrutped entity_addr_t to
  daemon, daemon will crash because decode fails and the exception is
  not handled. it's better than continuing working with the bogus
  message.

Signed-off-by: Kefu Chai <kchai@redhat.com>
(cherry picked from commit aa83c2c42dd52c8b1d459386309491feccfea567)

diff --git a/src/msg/async/AsyncConnection.cc b/src/msg/async/AsyncConnection.cc
index ea03f869074089d7a1d9d88bae2353ca3b79ed3d..8bb257235f42b81f7749e93d9f595855deb5f486 100644 (file)
--- a/src/msg/async/AsyncConnection.cc
+++ b/src/msg/async/AsyncConnection.cc
@@ -1275,10 +1275,13 @@ ssize_t AsyncConnection::_process_connection()
         }
 
         addr_bl.append(state_buffer+strlen(CEPH_BANNER), sizeof(ceph_entity_addr));
-        {
+        try {
           bufferlist::iterator ti = addr_bl.begin();
           ::decode(peer_addr, ti);
-        }
+        } catch (const buffer::error& e) {
+         lderr(async_msgr->cct) << __func__ <<  " decode peer_addr failed " << dendl;
+          goto fail;
+       }
 
         ldout(async_msgr->cct, 10) << __func__ << " accept peer addr is " << peer_addr << dendl;
         if (peer_addr.is_blank_ip()) {

diff --git a/src/msg/msg_types.h b/src/msg/msg_types.h
index 5632950f30918f40c0458014790fddc8b2c31dc4..1e048204d8bf67d45a58701a58b9d5256de8e047 100644 (file)
--- a/src/msg/msg_types.h
+++ b/src/msg/msg_types.h
@@ -454,7 +454,15 @@ struct entity_addr_t {
     __u32 elen;
     ::decode(elen, bl);
     if (elen) {
-      bl.copy(elen, (char*)get_sockaddr());
+      if (elen < sizeof(u.sa.sa_family)) {
+       throw buffer::malformed_input("elen smaller than family len");
+      }
+      bl.copy(sizeof(u.sa.sa_family), (char*)&u.sa.sa_family);
+      if (elen > get_sockaddr_len()) {
+       throw buffer::malformed_input("elen exceeds sockaddr len");
+      }
+      elen -= sizeof(u.sa.sa_family);
+      bl.copy(elen, u.sa.sa_data);
     }
     DECODE_FINISH(bl);
   }

diff --git a/src/msg/simple/Pipe.cc b/src/msg/simple/Pipe.cc
index eec90f5b02199846833f861c00e95afd0311739b..98c51be7cd2d9c64d41ffe2fe56b4b24f0da32b4 100644 (file)
--- a/src/msg/simple/Pipe.cc
+++ b/src/msg/simple/Pipe.cc
@@ -412,9 +412,13 @@ int Pipe::accept()
     ldout(msgr->cct,10) << "accept couldn't read peer_addr" << dendl;
     goto fail_unlocked;
   }
-  {
+  try {
     bufferlist::iterator ti = addrbl.begin();
     ::decode(peer_addr, ti);
+  } catch (const buffer::error& e) {
+    ldout(msgr->cct,2) << __func__ <<  " decode peer_addr failed: " << e.what()
+                       << dendl;
+    goto fail_unlocked;
   }
 
   ldout(msgr->cct,10) << "accept peer addr is " << peer_addr << dendl;