In XmppMessageBuilder::GetData() don't return temporary stack variabl…

…e address

In XmppMessageBuilder::GetData(), address of a string allocated in stack as
a temporary variable was returned to the caller in some case. This can cause
memory corruption because the the temporary string will get destroyed right
after it goes out of scope, in this case when we return to the caller from
GetData().

Fixed by ensuring that caller of GetData() provides the necessary temporary
storage space for the "temp" std::string

Change-Id: I27e622a34b72717cad90dc9553936182c2a77c6a
Closes-Bug: 1759744

src/bgp/bgp_message_builder.cc

@@ -261,7 +261,7 @@ void BgpMessage::Finish() {
 }
 
 const uint8_t *BgpMessage::GetData(IPeerUpdate *peer, size_t *lenp,
-    const string **msg_str) {
+    const string **msg_str, string *temp) {
     *lenp = datalen_;
     return data_;
 }

src/bgp/bgp_message_builder.h

@@ -21,7 +21,8 @@ class BgpMessage : public Message {
     virtual bool AddRoute(const BgpRoute *route, const RibOutAttr *roattr);
     virtual void Finish();
     virtual const uint8_t *GetData(IPeerUpdate *peer, size_t *lenp,
-                                   const std::string **msg_str);
+                                   const std::string **msg_str,
+                                   std::string *temp);
 
 private:
     virtual void Reset();

src/bgp/bgp_ribout_updates.cc

@@ -464,7 +464,8 @@ void RibOutUpdates::UpdateSend(int queue_id, Message *message,
         IPeerUpdate *peer = iter.Next();
         size_t msgsize = 0;
         const string *msg_str = NULL;
-        const uint8_t *data = message->GetData(peer, &msgsize, &msg_str);
+        string temp;
+        const uint8_t *data = message->GetData(peer, &msgsize, &msg_str, &temp);
         if (Sandesh::LoggingLevel() >= Sandesh::LoggingUtLevel()) {
             BGP_LOG_PEER(Message, peer, Sandesh::LoggingUtLevel(),
                 BGP_LOG_FLAG_SYSLOG, BGP_PEER_DIR_OUT,

src/bgp/message_builder.h

@@ -26,7 +26,7 @@ class Message {
     virtual bool AddRoute(const BgpRoute *route, const RibOutAttr *roattr) = 0;
     virtual void Finish() = 0;
     virtual const uint8_t *GetData(IPeerUpdate *peer_update, size_t *lenp,
-        const std::string **msg_str) = 0;
+        const std::string **msg_str, std::string *temp) = 0;
     uint64_t num_reach_routes() const { return num_reach_route_; }
     uint64_t num_unreach_routes() const { return num_unreach_route_; }
 

src/bgp/test/bgp_msg_builder_test.cc

@@ -117,7 +117,8 @@ TEST_F(BgpMsgBuilderTest, Build) {
 
     size_t length;
     const string *msg_str;
-    const uint8_t *data = message.GetData(NULL, &length, &msg_str);
+    string temp;
+    const uint8_t *data = message.GetData(NULL, &length, &msg_str, &temp);
     for (size_t i = 0; i < length; i++) {
         printf("%02x ", data[i]);
     }
@@ -152,7 +153,7 @@ TEST_F(BgpMsgBuilderTest, Build) {
     route2.InsertPath(path2);
     message.AddRoute(&route2, &rib_out_attr);
 
-    data = message.GetData(NULL, &length, &msg_str);
+    data = message.GetData(NULL, &length, &msg_str, &temp);
     for (size_t i = 0; i < length; i++) {
         printf("%02x ", data[i]);
     }

src/bgp/test/xmpp_message_builder_test.cc

@@ -214,7 +214,9 @@ TEST_P(XmppMessageBuilderParamTest, Basic) {
             XmppTestPeer peer(peer_name);
             size_t msgsize;
             const string *msg_str = NULL;
-            const uint8_t *msg = message_->GetData(&peer, &msgsize, &msg_str);
+            string temp;
+            const uint8_t *msg = message_->GetData(&peer, &msgsize, &msg_str,
+                                                   &temp);
             peer.SendUpdate(msg, msgsize, reuse_msg_str ? msg_str : NULL);
         }
     }
@@ -249,7 +251,9 @@ TEST_P(XmppMvpnMessageBuilderParamTest, Basic) {
             XmppTestPeer peer(peer_name);
             size_t msgsize;
             const string *msg_str = NULL;
-            const uint8_t *msg = message_->GetData(&peer, &msgsize, &msg_str);
+            string temp;
+            const uint8_t *msg = message_->GetData(&peer, &msgsize, &msg_str,
+                                                   &temp);
             peer.SendUpdate(msg, msgsize, reuse_msg_str ? msg_str : NULL);
         }
     }

src/bgp/xmpp_message_builder.cc

@@ -516,7 +516,7 @@ bool BgpXmppMessage::AddMvpnRoute(const BgpRoute *route,
 }
 
 const uint8_t *BgpXmppMessage::GetData(IPeerUpdate *peer, size_t *lenp,
-    const string **msg_str) {
+    const string **msg_str, string *temp) {
     // Build begin line that contains message opening tag with from and to
     // attributes.
     msg_begin_.clear();
@@ -551,10 +551,10 @@ const uint8_t *BgpXmppMessage::GetData(IPeerUpdate *peer, size_t *lenp,
         *msg_str = &repr_;
         return reinterpret_cast<const uint8_t *>(repr_.c_str()) + extra;
     } else {
-        string temp = msg_begin_ + string(repr_, kMaxFromToLength);
-        *lenp = temp.size();
+        *temp = msg_begin_ + string(repr_, kMaxFromToLength);
+        *lenp = temp->size();
         *msg_str = NULL;
-        return reinterpret_cast<const uint8_t *>(temp.c_str());
+        return reinterpret_cast<const uint8_t *>(temp->c_str());
     }
 }
 

src/bgp/xmpp_message_builder.h

@@ -40,7 +40,8 @@ class BgpXmppMessage : public Message {
     virtual void Finish();
     virtual bool AddRoute(const BgpRoute *route, const RibOutAttr *roattr);
     virtual const uint8_t *GetData(IPeerUpdate *peer, size_t *lenp,
-                                   const std::string **msg_str);
+                                   const std::string **msg_str,
+                                   std::string *temp);
 
 private:
     static const size_t kMaxFromToLength = 192;