From d624bcb1c17187215bfd95fc4e1a6b178c4bc3ee Mon Sep 17 00:00:00 2001
From: Jonathan Hui <jonhui@nestlabs.com>
Date: Fri, 4 Nov 2016 08:47:36 -0700
Subject: [PATCH] Fix buffer overrun issues when processing URI Paths. (#928)

---
 src/core/coap/coap_server.cpp     | 15 ++++++++++-----
 src/core/coap/coap_server.hpp     | 10 +++++-----
 src/core/meshcop/commissioner.cpp | 18 ++++++++++--------
 3 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/src/core/coap/coap_server.cpp b/src/core/coap/coap_server.cpp
index e5a6ac437..ec38069f1 100644
--- a/src/core/coap/coap_server.cpp
+++ b/src/core/coap/coap_server.cpp
@@ -109,7 +109,7 @@ void Server::HandleUdpReceive(void *aContext, otMessage aMessage, const otMessag
 void Server::HandleUdpReceive(Message &aMessage, const Ip6::MessageInfo &aMessageInfo)
 {
     Header header;
-    char uriPath[kMaxReceivedUriPath];
+    char uriPath[Resource::kMaxReceivedUriPath] = "";
     char *curUriPath = uriPath;
     const Header::Option *coapOption;
 
@@ -123,10 +123,15 @@ void Server::HandleUdpReceive(Message &aMessage, const Ip6::MessageInfo &aMessag
         switch (coapOption->mNumber)
         {
         case kCoapOptionUriPath:
-            VerifyOrExit(coapOption->mLength < sizeof(uriPath) - static_cast<size_t>(curUriPath - uriPath), ;);
+            if (curUriPath != uriPath)
+            {
+                *curUriPath++ = '/';
+            }
+
+            VerifyOrExit(coapOption->mLength < sizeof(uriPath) - static_cast<size_t>(curUriPath + 1 - uriPath), ;);
+
             memcpy(curUriPath, coapOption->mValue, coapOption->mLength);
-            curUriPath[coapOption->mLength] = '/';
-            curUriPath += coapOption->mLength + 1;
+            curUriPath += coapOption->mLength;
             break;
 
         case kCoapOptionContentFormat:
@@ -139,7 +144,7 @@ void Server::HandleUdpReceive(Message &aMessage, const Ip6::MessageInfo &aMessag
         coapOption = header.GetNextOption();
     }
 
-    curUriPath[-1] = '\0';
+    curUriPath[0] = '\0';
 
     for (Resource *resource = mResources; resource; resource = resource->mNext)
     {
diff --git a/src/core/coap/coap_server.hpp b/src/core/coap/coap_server.hpp
index 65d1da3ea..b3d82f73a 100644
--- a/src/core/coap/coap_server.hpp
+++ b/src/core/coap/coap_server.hpp
@@ -57,6 +57,11 @@ class Resource
     friend class Server;
 
 public:
+    enum
+    {
+        kMaxReceivedUriPath = 32,   ///< Maximum supported URI path on received messages.
+    };
+
     /**
      * This function pointer is called when a CoAP message with a given Uri-Path is received.
      *
@@ -168,11 +173,6 @@ public:
     ThreadError SendMessage(Message &aMessage, const Ip6::MessageInfo &aMessageInfo);
 
 private:
-    enum
-    {
-        kMaxReceivedUriPath = 32,   ///< Maximum supported URI path on received messages.
-    };
-
     static void HandleUdpReceive(void *aContext, otMessage aMessage, const otMessageInfo *aMessageInfo);
     void HandleUdpReceive(Message &aMessage, const Ip6::MessageInfo &aMessageInfo);
 
diff --git a/src/core/meshcop/commissioner.cpp b/src/core/meshcop/commissioner.cpp
index 3d09579de..108da195f 100644
--- a/src/core/meshcop/commissioner.cpp
+++ b/src/core/meshcop/commissioner.cpp
@@ -868,7 +868,7 @@ void Commissioner::ReceiveJoinerFinalize(uint8_t *buf, uint16_t length)
 {
     Message *message = NULL;
     Coap::Header header;
-    char uriPath[16];
+    char uriPath[Coap::Resource::kMaxReceivedUriPath] = "";
     char *curUriPath = uriPath;
     const Coap::Header::Option *coapOption;
 
@@ -890,10 +890,15 @@ void Commissioner::ReceiveJoinerFinalize(uint8_t *buf, uint16_t length)
         switch (coapOption->mNumber)
         {
         case kCoapOptionUriPath:
-            VerifyOrExit(coapOption->mLength < sizeof(uriPath) - static_cast<uint16_t>(curUriPath - uriPath), ;);
+            if (curUriPath != uriPath)
+            {
+                *curUriPath++ = '/';
+            }
+
+            VerifyOrExit(coapOption->mLength < sizeof(uriPath) - static_cast<size_t>(curUriPath + 1 - uriPath), ;);
+
             memcpy(curUriPath, coapOption->mValue, coapOption->mLength);
-            curUriPath[coapOption->mLength] = '/';
-            curUriPath += coapOption->mLength + 1;
+            curUriPath += coapOption->mLength;
             break;
 
         case kCoapOptionContentFormat:
@@ -906,10 +911,7 @@ void Commissioner::ReceiveJoinerFinalize(uint8_t *buf, uint16_t length)
         coapOption = header.GetNextOption();
     }
 
-    if (curUriPath > uriPath)
-    {
-        curUriPath[-1] = '\0';
-    }
+    curUriPath[0] = '\0';
 
     VerifyOrExit(strcmp(uriPath, OPENTHREAD_URI_JOINER_FINALIZE) == 0,);