From: Kefu Chai <kchai@redhat.com>
Date: Wed, 13 Sep 2017 14:55:58 +0000 (+0800)
Subject: msg/msg_types: fix the entity_addr_t's decoder
X-Git-Tag: v12.2.12~72^2~1
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=d174210f7e9c56f6ea85993863f11d4f84c6a9c3;p=ceph.git

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 ea03f8690740..8bb257235f42 100644
--- 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 5632950f3091..1e048204d8bf 100644
--- 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 eec90f5b0219..98c51be7cd2d 100644
--- 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;