From aa83c2c42dd52c8b1d459386309491feccfea567 Mon Sep 17 00:00:00 2001
From: Kefu Chai <kchai@redhat.com>
Date: Wed, 13 Sep 2017 22:55:58 +0800
Subject: [PATCH] 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>
---
 src/msg/async/AsyncConnection.cc |  7 +++++--
 src/msg/msg_types.h              | 10 +++++++++-
 src/msg/simple/Pipe.cc           |  6 +++++-
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/src/msg/async/AsyncConnection.cc b/src/msg/async/AsyncConnection.cc
index 7899a172759d0..ce76f1508834c 100644
--- a/src/msg/async/AsyncConnection.cc
+++ b/src/msg/async/AsyncConnection.cc
@@ -1261,10 +1261,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 8a20ba0f060eb..f98d9600f7a7f 100644
--- a/src/msg/msg_types.h
+++ b/src/msg/msg_types.h
@@ -456,7 +456,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 4a7ab9acab7a0..3e4293cb49eff 100644
--- a/src/msg/simple/Pipe.cc
+++ b/src/msg/simple/Pipe.cc
@@ -407,9 +407,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;
-- 
2.39.5