[IOT-1513] Incorrect URI construction

The % character is not legal to be placed literally in a URI, it must be
escaped (as "%25") before being enclosed in a URI.  Other places were
changed in July in change 9419, and some more in change 14009, but these
code paths were not changed at that time.

This also fixes a bug in setHost which did not mark an IPv6 address as
IPv6, and adds a test case that covers this.

Change-Id: I1331ecb9c5482a2d43dd675978a1f34c6d37cb4c
Signed-off-by: Dave Thaler <dthaler@microsoft.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/14263Reviewed-by: jihwan seo <jihwan.seo@samsung.com>
Tested-by: jenkins-iotivity <jenkins-iotivity@opendaylight.org>
Reviewed-by: Ashwini  Kumar <k.ashwini@samsung.com>
Reviewed-by: Dan Mihai <Daniel.Mihai@microsoft.com>

resource/csdk/octbstack_product.def

@@ -37,6 +37,7 @@ OCBindResourceTypeToResource
 OCCancel
 OCCreateOCStringLL
 OCCreateResource
+OCDecodeAddressForRFC6874
 OCDeleteResource
 OCDiscoverDirectPairingDevices
 OCDiscoveryPayloadCreate
@@ -45,6 +46,7 @@ OCDiscoveryPayloadGetResourceCount
 OCDoDirectPairing
 OCDoResource
 OCDoResponse
+OCEncodeAddressForRFC6874
 OCFreeOCStringLL
 OCGetDirectPairedDevices
 OCGetNumberOfResources

resource/csdk/security/provisioning/src/pmutility.c

@@ -474,13 +474,26 @@ bool PMGenerateQuery(bool isSecure,
             switch(connType & CT_MASK_FLAGS & ~CT_FLAG_SECURE)
             {
                 case CT_IP_USE_V4:
-                        snRet = snprintf(buffer, bufferSize, "%s%s:%d%s",
-                                         prefix, address, port, uri);
+                    snRet = snprintf(buffer, bufferSize, "%s%s:%d%s",
+                                     prefix, address, port, uri);
                     break;
                 case CT_IP_USE_V6:
-                        snRet = snprintf(buffer, bufferSize, "%s[%s]:%d%s",
-                                         prefix, address, port, uri);
+                {
+                    char addressEncoded[128] = {0};
+
+                    OCStackResult result = OCEncodeAddressForRFC6874(addressEncoded,
+                                                                     sizeof(addressEncoded),
+                                                                     address);
+                    if (OC_STACK_OK != result)
+                    {
+                        OIC_LOG_V(ERROR, TAG, "PMGenerateQuery : encoding error %d\n", result);
+                        return false;
+                    }
+
+                    snRet = snprintf(buffer, bufferSize, "%s[%s]:%d%s",
+                                     prefix, addressEncoded, port, uri);
                     break;
+                }
                 default:
                     OIC_LOG(ERROR, TAG, "Unknown address format.");
                     return false;

resource/csdk/security/provisioning/src/secureresourceprovider.c

@@ -1208,6 +1208,16 @@ static OCStackResult SendDeleteCredentialRequest(void* ctx,
         return OC_STACK_ERROR;
     }
 
+    char addressEncoded[CA_MAX_URI_LENGTH] = {0};
+    OCStackResult result = OCEncodeAddressForRFC6874(addressEncoded,
+                                                     sizeof(addressEncoded),
+                                                     destDev->endpoint.addr);
+    if (OC_STACK_OK != result)
+    {
+        OIC_LOG_V(ERROR, TAG, "SendDeleteCredentialRequest : encoding error %d\n", result);
+        return OC_STACK_ERROR;
+    }
+
     char reqBuf[MAX_URI_LENGTH + MAX_QUERY_LENGTH] = {0};
     int snRet = 0;
                     //coaps://0.0.0.0:5684/oic/sec/cred?subjectid=(Canonical ENCODED UUID)
@@ -1218,7 +1228,8 @@ static OCStackResult SendDeleteCredentialRequest(void* ctx,
         srpUri = SRP_FORM_DELETE_CREDENTIAL_TCP;
     }
 #endif
-    snRet = snprintf(reqBuf, sizeof(reqBuf), srpUri, destDev->endpoint.addr,
+
+    snRet = snprintf(reqBuf, sizeof(reqBuf), srpUri, addressEncoded,
                      destDev->securePort, OIC_RSRC_CRED_URI, OIC_JSON_SUBJECTID_NAME, subID);
     OICFree(subID);
     if (snRet < 0)
@@ -1273,10 +1284,21 @@ static OCStackResult SendDeleteACLRequest(void* ctx,
         return OC_STACK_ERROR;
     }
 
+    char addressEncoded[CA_MAX_URI_LENGTH] = {0};
+    OCStackResult result = OCEncodeAddressForRFC6874(addressEncoded,
+                                                     sizeof(addressEncoded),
+                                                     destDev->endpoint.addr);
+    if (OC_STACK_OK != result)
+    {
+        OIC_LOG_V(ERROR, TAG, "SendDeleteCredentialRequest : encoding error %d\n", result);
+        return OC_STACK_ERROR;
+    }
+
+
     char reqBuf[MAX_URI_LENGTH + MAX_QUERY_LENGTH] = {0};
     int snRet = 0;
                     //coaps://0.0.0.0:5684/oic/sec/acl?subjectuuid=(Canonical ENCODED UUID)
-    snRet = snprintf(reqBuf, sizeof(reqBuf), SRP_FORM_DELETE_CREDENTIAL, destDev->endpoint.addr,
+    snRet = snprintf(reqBuf, sizeof(reqBuf), SRP_FORM_DELETE_CREDENTIAL, addressEncoded,
                      destDev->securePort, OIC_RSRC_ACL_URI, OIC_JSON_SUBJECTID_NAME, subID);
     OICFree(subID);
     if (snRet < 0)

resource/csdk/security/src/directpairing.c

@@ -266,13 +266,26 @@ bool DPGenerateQuery(bool isSecure,
             switch(connType & CT_MASK_FLAGS & ~CT_FLAG_SECURE)
             {
                 case CT_IP_USE_V4:
-                        snRet = snprintf(buffer, bufferSize, "%s%s:%d%s",
-                                         prefix, address, port, uri);
+                    snRet = snprintf(buffer, bufferSize, "%s%s:%d%s",
+                                     prefix, address, port, uri);
                     break;
                 case CT_IP_USE_V6:
-                        snRet = snprintf(buffer, bufferSize, "%s[%s]:%d%s",
-                                         prefix, address, port, uri);
+                {
+                    char addressEncoded[CA_MAX_URI_LENGTH] = {0};
+
+                    OCStackResult result = OCEncodeAddressForRFC6874(addressEncoded,
+                                                                     sizeof(addressEncoded),
+                                                                     address);
+                    if (OC_STACK_OK != result)
+                    {
+                        OIC_LOG_V(ERROR, TAG, "DPGenerateQuery : encoding error %d\n", result);
+                        return false;
+                    }
+
+                    snRet = snprintf(buffer, bufferSize, "%s[%s]:%d%s",
+                                     prefix, addressEncoded, port, uri);
                     break;
+                }
                 default:
                     OIC_LOG(ERROR, TAG, "Unknown address format.");
                     return false;

resource/csdk/stack/include/internal/ocstackinternal.h

@@ -283,10 +283,6 @@ const char *convertTriggerEnumToString(OCPresenceTrigger trigger);
 
 OCPresenceTrigger convertTriggerStringToEnum(const char * triggerStr);
 
-OCStackResult encodeAddressForRFC6874(char * outputAddress,
-                                      size_t outputSize,
-                                      const char * inputAddress);
-
 void CopyEndpointToDevAddr(const CAEndpoint_t *in, OCDevAddr *out);
 
 void CopyDevAddrToEndpoint(const OCDevAddr *in, CAEndpoint_t *out);

resource/csdk/stack/include/ocstack.h

@@ -666,6 +666,42 @@ OCStackResult OCGetDeviceId(OCUUIdentity *deviceId);
  * @return Returns ::OC_STACK_OK if success.
  */
 OCStackResult OCSetDeviceId(const OCUUIdentity *deviceId);
+
+/**
+ * Encode an address string to match RFC 6874.
+ *
+ * @param outputAddress    a char array to be written with the encoded string.
+ *
+ * @param outputSize       size of outputAddress buffer.
+ *
+ * @param inputAddress     a char array of size <= CA_MAX_URI_LENGTH
+ *                         containing a valid IPv6 address string.
+ *
+ * @return ::OC_STACK_OK on success and other value otherwise.
+ */
+OCStackResult OCEncodeAddressForRFC6874(char* outputAddress,
+                                        size_t outputSize,
+                                        const char* inputAddress);
+
+/**
+ * Decode an address string according to RFC 6874.
+ *
+ * @param outputAddress    a char array to be written with the decoded string.
+ *
+ * @param outputSize       size of outputAddress buffer.
+ *
+ * @param inputAddress     a valid percent-encoded address string.
+ *
+ * @param end              NULL if the entire entire inputAddress is a null-terminated percent-
+ *                         encoded address string.  Otherwise, a pointer to the first byte that
+ *                         is not part of the address string (e.g., ']' in a URI).
+ *
+ * @return ::OC_STACK_OK on success and other value otherwise.
+ */
+OCStackResult OCDecodeAddressForRFC6874(char* outputAddress,
+                                        size_t outputSize,
+                                        const char* inputAddress,
+                                        const char* end);
 #ifdef __cplusplus
 }
 #endif // __cplusplus

resource/csdk/stack/src/ocstack.c

@@ -868,22 +868,9 @@ OCPresenceTrigger convertTriggerStringToEnum(const char * triggerStr)
     }
 }
 
-/**
- * Encode an address string to match RFC 6874.
- *
- * @param outputAddress    a char array to be written with the encoded string.
- *
- * @param outputSize       size of outputAddress buffer.
- *
- * @param inputAddress     a char array of size <= CA_MAX_URI_LENGTH
- *                         containing a valid IPv6 address string.
- *
- * @return                 OC_STACK_OK if encoding succeeded.
- *                         Else an error occured.
- */
- OCStackResult encodeAddressForRFC6874(char *outputAddress,
-                                       size_t outputSize,
-                                       const char *inputAddress)
+OCStackResult OCEncodeAddressForRFC6874(char *outputAddress,
+                                        size_t outputSize,
+                                        const char *inputAddress)
 {
     VERIFY_NON_NULL(inputAddress,  FATAL, OC_STACK_INVALID_PARAM);
     VERIFY_NON_NULL(outputAddress, FATAL, OC_STACK_INVALID_PARAM);
@@ -896,7 +883,7 @@ OCPresenceTrigger convertTriggerStringToEnum(const char * triggerStr)
     if (inputSize > outputSize)
     {
         OIC_LOG_V(ERROR, TAG,
-                  "encodeAddressForRFC6874 failed: "
+                  "OCEncodeAddressForRFC6874 failed: "
                   "outputSize (%zu) < inputSize (%zu)",
                   outputSize, inputSize);
 
@@ -924,21 +911,21 @@ OCPresenceTrigger convertTriggerStringToEnum(const char * triggerStr)
     // If no string follows the first '%', then the input was invalid.
     if (scopeIdPart[0] == '\0')
     {
-        OIC_LOG(ERROR, TAG, "encodeAddressForRFC6874 failed: Invalid input string: no scope ID!");
+        OIC_LOG(ERROR, TAG, "OCEncodeAddressForRFC6874 failed: Invalid input string: no scope ID!");
         return OC_STACK_ERROR;
     }
 
     // Check to see if the string is already encoded
     if ((scopeIdPart[0] == '2') && (scopeIdPart[1] == '5'))
     {
-        OIC_LOG(ERROR, TAG, "encodeAddressForRFC6874 failed: Input string is already encoded");
+        OIC_LOG(ERROR, TAG, "OCEncodeAddressForRFC6874 failed: Input string is already encoded");
         return OC_STACK_ERROR;
     }
 
     // Fail if we don't have room for encoded string's two additional chars
     if (outputSize < (inputSize + 2))
     {
-        OIC_LOG(ERROR, TAG, "encodeAddressForRFC6874 failed: encoded output will not fit!");
+        OIC_LOG(ERROR, TAG, "OCEncodeAddressForRFC6874 failed: encoded output will not fit!");
         return OC_STACK_ERROR;
     }
 
@@ -950,6 +937,41 @@ OCPresenceTrigger convertTriggerStringToEnum(const char * triggerStr)
     return OC_STACK_OK;
 }
 
+OCStackResult OCDecodeAddressForRFC6874(char *outputAddress,
+                                        size_t outputSize,
+                                        const char *inputAddress,
+                                        const char *end)
+{
+    VERIFY_NON_NULL(inputAddress,  FATAL, OC_STACK_INVALID_PARAM);
+    VERIFY_NON_NULL(outputAddress, FATAL, OC_STACK_INVALID_PARAM);
+
+    if (NULL == end)
+    {
+        end = inputAddress + strlen(inputAddress);
+    }
+    size_t inputLength = end - inputAddress;
+
+    const char *percent = strchr(inputAddress, '%');
+    if (!percent || (percent > end))
+    {
+        OICStrcpyPartial(outputAddress, outputSize, inputAddress, inputLength);
+    }
+    else
+    {
+        if (percent[1] != '2' || percent[2] != '5')
+        {
+            return OC_STACK_INVALID_URI;
+        }
+
+        size_t addrlen = percent - inputAddress + 1;
+        OICStrcpyPartial(outputAddress, outputSize, inputAddress, addrlen);
+        OICStrcpyPartial(outputAddress + addrlen, outputSize - addrlen,
+                         percent + 3, end - percent - 3);
+    }
+
+    return OC_STACK_OK;
+}
+
 /**
  * The cononical presence allows constructed URIs to be string compared.
  *
@@ -980,9 +1002,9 @@ static int FormCanonicalPresenceUri(const CAEndpoint_t *endpoint,
             {
                 char addressEncoded[CA_MAX_URI_LENGTH] = {0};
 
-                OCStackResult result = encodeAddressForRFC6874(addressEncoded,
-                                                               sizeof(addressEncoded),
-                                                               ep->addr);
+                OCStackResult result = OCEncodeAddressForRFC6874(addressEncoded,
+                                                                 sizeof(addressEncoded),
+                                                                 ep->addr);
 
                 if (OC_STACK_OK != result)
                 {
@@ -2515,23 +2537,11 @@ static OCStackResult ParseRequestUri(const char *fullUri,
         }
 
         // Decode address per RFC 6874.
-        char *percent = strchr(start, '%');
-        if (!percent || (percent > end))
-        {
-            OICStrcpyPartial(da->addr, sizeof(da->addr), start, len);
-        }
-        else
+        result = OCDecodeAddressForRFC6874(da->addr, sizeof(da->addr), start, end);
+        if (result != OC_STACK_OK)
         {
-            if (percent[1] != '2' || percent[2] != '5')
-            {
-                OICFree(*devAddr);
-                return OC_STACK_INVALID_URI;
-            }
-
-            int addrlen = percent - start + 1;
-            OICStrcpyPartial(da->addr, sizeof(da->addr), start, addrlen);
-            OICStrcpyPartial(da->addr + addrlen, sizeof(da->addr) - addrlen,
-                             percent + 3, end - percent - 3);
+             OICFree(*devAddr);
+             return result;
         }
 
         da->port = port;

resource/csdk/stack/test/stacktests.cpp

@@ -1815,7 +1815,7 @@ TEST(StackUri, Rfc6874_Noop_1)
     char bytes[100] = {0};
     strncpy(bytes, validIPv6Address, sizeof(bytes));
 
-    OCStackResult result = encodeAddressForRFC6874(bytes, sizeof(bytes), validIPv6Address);
+    OCStackResult result = OCEncodeAddressForRFC6874(bytes, sizeof(bytes), validIPv6Address);
 
     // No % sign, should do nothing
     EXPECT_STREQ(bytes, validIPv6Address);
@@ -1827,7 +1827,7 @@ TEST(StackUri, Rfc6874_Noop_2)
     char validIPv6Address[] = "3812:a61::4:1";
     char bytes[100] = {0};
 
-    OCStackResult result = encodeAddressForRFC6874(bytes, sizeof(bytes), validIPv6Address);
+    OCStackResult result = OCEncodeAddressForRFC6874(bytes, sizeof(bytes), validIPv6Address);
 
     // No % sign, should do nothing
     EXPECT_STREQ(bytes, validIPv6Address);
@@ -1841,7 +1841,7 @@ TEST(StackUri, Rfc6874_WithEncoding)
     char bytes[100] = "";
     strncpy(bytes, validIPv6Address, sizeof(bytes));
 
-    OCStackResult result = encodeAddressForRFC6874(bytes, sizeof(bytes), validIPv6Address);
+    OCStackResult result = OCEncodeAddressForRFC6874(bytes, sizeof(bytes), validIPv6Address);
 
     // Encoding should have occured
     EXPECT_STREQ(bytes, validIPv6AddressEncoded);
@@ -1853,7 +1853,7 @@ TEST(StackUri, Rfc6874_WithEncoding_ExtraPercent)
     char validIPv6Address[] = "fe80::dafe:e3ff:fe00:ebfa%%wlan0";
     char bytes[100] = {0};
 
-    OCStackResult result = encodeAddressForRFC6874(bytes, sizeof(bytes), validIPv6Address);
+    OCStackResult result = OCEncodeAddressForRFC6874(bytes, sizeof(bytes), validIPv6Address);
 
     // Encoding should have failed due to extra '%' character
     EXPECT_STREQ(bytes, "");
@@ -1865,7 +1865,7 @@ TEST(StackUri, Rfc6874_AlreadyEncoded)
     char validIPv6AddressEncoded[] = "fe80::dafe:e3ff:fe00:ebfa%25wlan0";
     char bytes[100] = {0};
 
-    OCStackResult result = encodeAddressForRFC6874(bytes, sizeof(bytes), validIPv6AddressEncoded);
+    OCStackResult result = OCEncodeAddressForRFC6874(bytes, sizeof(bytes), validIPv6AddressEncoded);
 
     // Encoding should have failed due to extra '%' character
     EXPECT_STREQ(bytes, "");
@@ -1883,7 +1883,7 @@ TEST(StackUri, Rfc6874_NoOverflow)
     addrBuffer[sizeof(addrBuffer) - sizeof(validIPv6Address) - 3] = '\0';
     strcat(addrBuffer, validIPv6Address);
 
-    OCStackResult result = encodeAddressForRFC6874(bytes, sizeof(bytes), addrBuffer);
+    OCStackResult result = OCEncodeAddressForRFC6874(bytes, sizeof(bytes), addrBuffer);
 
     // Encoding should have succeeded
     EXPECT_EQ(OC_STACK_OK, result);
@@ -1900,7 +1900,7 @@ TEST(StackUri, Rfc6874_NoOverflow_2)
     addrBuffer[sizeof(addrBuffer) - sizeof(validIPv6Address) - 1] = '\0';
     strcat(addrBuffer, validIPv6Address);
 
-    OCStackResult result = encodeAddressForRFC6874(bytes, sizeof(bytes), addrBuffer);
+    OCStackResult result = OCEncodeAddressForRFC6874(bytes, sizeof(bytes), addrBuffer);
 
     // Encoding should have failed due to output size limitations
     EXPECT_STREQ(bytes, "");

resource/src/OCResource.cpp

@@ -33,6 +33,7 @@
 #ifdef HAVE_IN6ADDR_H
 #include <in6addr.h>
 #endif
+#include "ocstack.h"
 
 namespace OC {
 
@@ -199,10 +200,17 @@ void OCResource::setHost(const std::string& host)
                 m_interfaces.empty(), m_clientWrapper.expired(), false, false);
         }
 
-        ip6Addr.copy(m_devAddr.addr, sizeof(m_devAddr.addr));
-        m_devAddr.addr[ip6Addr.length()] = '\0';
+        OCStackResult result = OCDecodeAddressForRFC6874(m_devAddr.addr,
+            sizeof(m_devAddr.addr), ip6Addr.c_str(), nullptr);
+
+        if (OC_STACK_OK != result)
+        {
+            throw ResourceInitException(m_uri.empty(), m_resourceTypes.empty(),
+                m_interfaces.empty(), m_clientWrapper.expired(), false, false);
+        }
+
         m_devAddr.port = static_cast<uint16_t>(port);
-        m_devAddr.flags = static_cast<OCTransportFlags>(m_devAddr.flags & OC_IP_USE_V6);
+        m_devAddr.flags = static_cast<OCTransportFlags>(m_devAddr.flags | OC_IP_USE_V6);
     }
     else if (host_token[0] == ':')
     {
@@ -539,7 +547,17 @@ std::string OCResource::host() const
 
     if (m_devAddr.flags & OC_IP_USE_V6)
     {
-        ss << '[' << m_devAddr.addr << ']';
+        char addressEncoded[128] = {0};
+
+        OCStackResult result = OCEncodeAddressForRFC6874(addressEncoded,
+                                                         sizeof(addressEncoded),
+                                                         m_devAddr.addr);
+        if (OC_STACK_OK != result)
+        {
+            throw ResourceInitException(m_uri.empty(), m_resourceTypes.empty(),
+                m_interfaces.empty(), m_clientWrapper.expired(), false, false);
+        }
+        ss << '[' << addressEncoded << ']';
     }
     else
     {

resource/unittests/OCResourceTest.cpp

@@ -80,7 +80,7 @@ namespace OCResourceTest
 
     TEST(ConstructResourceTest, ConstructResourceObjectWithValidHost3)
     {
-        EXPECT_NO_THROW(ConstructResourceObject("coap://[ffff::ffff%eth0]:5000", "/resource"));
+        EXPECT_NO_THROW(ConstructResourceObject("coap://[ffff::ffff%25eth0]:5000", "/resource"));
     }
 
     TEST(ConstructResourceTest, ConstructResourceObject)
@@ -123,6 +123,11 @@ namespace OCResourceTest
         EXPECT_ANY_THROW(ConstructResourceObject("coap://[ffff:::ffff]:5000", "/resource"));
     }
 
+    TEST(ConstructResourceTest, ConstructResourceObjectInvalidHost4)
+    {
+        EXPECT_ANY_THROW(ConstructResourceObject("coap://[ffff::ffff%eth0]:5000", "/resource"));
+    }
+
     TEST(ConstructResourceTest, ConstructResourceObjectInvalidUri)
     {
         EXPECT_ANY_THROW(ConstructResourceObject("coap://192.168.1.2:5000", "/"));
@@ -500,6 +505,13 @@ namespace OCResourceTest
         EXPECT_TRUE(resource->host() == "coap://192.168.1.2:5000");
     }
 
+    TEST(HostTest, Host2)
+    {
+        OCResource::Ptr resource = ConstructResourceObject("coap://[ffff::ffff%25eth0]:5000", "/resource");
+        EXPECT_TRUE(resource != NULL);
+        EXPECT_TRUE(resource->host() == "coap://[ffff::ffff%25eth0]:5000");
+    }
+
     //Uri Test
     TEST(UriTest, Uri)
     {

service/scene-manager/src/SceneUtils.cpp

@@ -95,7 +95,18 @@ namespace OIC
                                             + std::to_string(netInfo[i].port);
                     } else if(netInfo[i].flags == CATransportFlags_t::CA_IPV6)
                     {
-                        address_ipv6 = "[" + std::string(netInfo[i].addr) + "]:"
+                        char addressEncoded[CA_MAX_URI_LENGTH] = {0};
+
+                        OCStackResult result = OCEncodeAddressForRFC6874(addressEncoded,
+                                                                         sizeof(addressEncoded),
+                                                                         netInfo[i].addr);
+                        if (OC_STACK_OK != result)
+                        {
+                            OICFree(netInfo);
+                            throw RCSException("address encoding error");
+                        }
+
+                        address_ipv6 = "[" + std::string(addressEncoded) + "]:"
                                             + std::to_string(netInfo[i].port);
                     }
                 }