Merge pull request #9341 from bergzand/pr/sockutil/cleanup

sock_util: Add unittest and fix detected issues.

sys/include/net/sock/util.h

@@ -26,6 +26,7 @@
 #define NET_SOCK_UTIL_H
 
 #include <stdbool.h>
+#include <stdint.h>
 
 #include "net/sock/udp.h"
 
@@ -52,8 +53,9 @@ int sock_udp_ep_fmt(const sock_udp_ep_t *endpoint, char *addr_str, uint16_t *por
  * "host.name:1234" and "/url/path".
  *
  * @note Caller has to make sure hostport and urlpath can hold the results!
- *       Make sure to provide space for SOCK_HOSTPORT_MAXLEN respectively
- *       SOCK_URLPATH_MAXLEN bytes.
+ *       Make sure to provide space for @ref SOCK_HOSTPORT_MAXLEN respectively
+ *       @ref SOCK_URLPATH_MAXLEN bytes.
+ *       Scheme part of the URL is limited to @ref SOCK_SCHEME_MAXLEN length.
  *
  * @param[in]   url         URL to split
  * @param[out]  hostport    where to write host:port
@@ -97,6 +99,9 @@ bool sock_udp_ep_equal(const sock_udp_ep_t *a, const sock_udp_ep_t *b);
  * @name helper definitions
  * @{
  */
+#define SOCK_SCHEME_MAXLEN      (16U)   /**< maximum length of the scheme part
+                                             for sock_urlsplit. Ensures a hard
+                                             limit on the string iterator */
 #define SOCK_HOSTPORT_MAXLEN    (64U)   /**< maximum length of host:port part for
                                              sock_urlsplit() */
 #define SOCK_URLPATH_MAXLEN     (64U)   /**< maximum length path for

sys/net/sock/sock_util.c

@@ -22,6 +22,7 @@
 #include <arpa/inet.h>
 #include <errno.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 
 #include "net/sock/udp.h"
@@ -31,9 +32,6 @@
 #include "fmt.h"
 #endif
 
-#define SOCK_HOST_MAXLEN    (64U)   /**< maximum length of host part for
-                                         sock_udp_str2ep() */
-
 int sock_udp_ep_fmt(const sock_udp_ep_t *endpoint, char *addr_str, uint16_t *port)
 {
     void *addr_ptr;
@@ -68,7 +66,7 @@ int sock_udp_ep_fmt(const sock_udp_ep_t *endpoint, char *addr_str, uint16_t *por
         char *tmp = addr_str + strlen(addr_str);
         *tmp++ = '%';
         tmp += fmt_u16_dec(tmp, endpoint->netif);
-        *tmp = '0';
+        *tmp = '\0';
 #else
         sprintf(addr_str + strlen(addr_str), "%%%4u", endpoint->netif);
 #endif
@@ -84,8 +82,13 @@ int sock_udp_ep_fmt(const sock_udp_ep_t *endpoint, char *addr_str, uint16_t *por
 
 static char* _find_hoststart(const char *url)
 {
+    /* Increment SOCK_SCHEME_MAXLEN due to comparison with the colon after the
+     * scheme part
+     */
+    size_t remaining = SOCK_SCHEME_MAXLEN + 1;
     char *urlpos = (char*)url;
-    while(*urlpos) {
+    while(*urlpos && remaining) {
+        remaining--;
         if (*urlpos++ == ':') {
             if (strncmp(urlpos, "//", 2) == 0) {
                 return urlpos + 2;
@@ -99,14 +102,16 @@ static char* _find_hoststart(const char *url)
 
 static char* _find_pathstart(const char *url)
 {
+    size_t remaining = SOCK_HOSTPORT_MAXLEN;
     char *urlpos = (char*)url;
-    while(*urlpos) {
+    while(*urlpos && remaining) {
+        remaining--;
         if (*urlpos == '/') {
             return urlpos;
         }
         urlpos++;
     }
-    return NULL;
+    return urlpos;
 }
 
 int sock_urlsplit(const char *url, char *hostport, char *urlpath)
@@ -117,19 +122,24 @@ int sock_urlsplit(const char *url, char *hostport, char *urlpath)
     }
 
     char *pathstart = _find_pathstart(hoststart);
-    if(!pathstart) {
-        return -EINVAL;
-    }
 
-    memcpy(hostport, hoststart, pathstart - hoststart);
+    size_t hostlen = pathstart - hoststart;
+    /* hostlen must be smaller SOCK_HOSTPORT_MAXLEN to have space for the null
+     * terminator */
+    if (hostlen > SOCK_HOSTPORT_MAXLEN - 1) {
+        return -EOVERFLOW;
+    }
+    memcpy(hostport, hoststart, hostlen);
+    *(hostport + hostlen) = '\0';
 
     size_t pathlen = strlen(pathstart);
     if (pathlen) {
+        if (pathlen > SOCK_URLPATH_MAXLEN - 1) {
+            return -EOVERFLOW;
+        }
         memcpy(urlpath, pathstart, pathlen);
     }
-    else {
-        *urlpath = '\0';
-    }
+    *(urlpath + pathlen) = '\0';
     return 0;
 }
 
@@ -139,7 +149,7 @@ int sock_udp_str2ep(sock_udp_ep_t *ep_out, const char *str)
     char *hoststart = (char*)str;
     char *hostend = hoststart;
 
-    char hostbuf[SOCK_HOST_MAXLEN];
+    char hostbuf[SOCK_HOSTPORT_MAXLEN];
 
     memset(ep_out, 0, sizeof(sock_udp_ep_t));
 
@@ -147,22 +157,32 @@ int sock_udp_str2ep(sock_udp_ep_t *ep_out, const char *str)
         brackets_flag = 1;
         for (hostend = ++hoststart; *hostend && *hostend != ']';
                 hostend++);
-        if (! *hostend) {
+        if (! *hostend || ((size_t)(hostend - hoststart) >= sizeof(hostbuf))) {
             /* none found, bail out */
             return -EINVAL;
         }
     }
     else {
         brackets_flag = 0;
-        for (hostend = hoststart; *hostend && *hostend != ':';
-                hostend++);
+        for (hostend = hoststart; *hostend && (*hostend != ':') && \
+                ((size_t)(hostend - hoststart) < sizeof(hostbuf)); hostend++) {}
     }
 
+    size_t hostlen = hostend - hoststart;
     if (*(hostend + brackets_flag) == ':') {
-        ep_out->port = atoi(hostend + brackets_flag + 1);
+        char *portstart = hostend + brackets_flag + 1;
+        /* Checks here verify that the supplied port number is up to 5 (random)
+         * chars in size and result is smaller or equal to UINT16_MAX. */
+        if (strlen(portstart) > 5) {
+            return -EINVAL;
+        }
+        uint32_t port = atol(portstart);
+        if (port > UINT16_MAX) {
+            return -EINVAL;
+        }
+        ep_out->port = (uint16_t)port;
     }
 
-    size_t hostlen = hostend - hoststart;
     if (hostlen >= sizeof(hostbuf)) {
         return -EINVAL;
     }

tests/unittests/tests-sock_util/Makefile

+include $(RIOTBASE)/Makefile.base

tests/unittests/tests-sock_util/Makefile.include

+USEMODULE += sock_util
+USEMODULE += gnrc_sock
+USEMODULE += gnrc_ipv6
+USEMODULE += ipv4_addr
+USEMODULE += ipv6_addr
+USEMODULE += posix

tests/unittests/tests-sock_util/tests-sock_util.c

+/*
+ * Copyright (C) 2018 Freie Universität Berlin
+ * Copyright (C) 2018 Inria
+ *
+ * This file is subject to the terms and conditions of the GNU Lesser
+ * General Public License v2.1. See the file LICENSE in the top level
+ * directory for more details.
+ */
+
+/**
+ * @ingroup    tests
+ * @{
+ *
+ * @file
+ * @brief      Unit tests for sock_util module
+ *
+ * @author     Koen Zandberg <koen@bergzand.net>
+ */
+
+#include "embUnit.h"
+#include "net/sock/util.h"
+#include "stdio.h"
+
+#define TEST_IPV6_ADDR      { 0x20, 0x01, 0x0d, 0xb8, \
+                              0x00, 0x00, 0x00, 0x00, \
+                              0x00, 0x00, 0x00, 0x00, \
+                              0x00, 0x00, 0x00, 0x01 }
+#define TEST_IPV6_NETIF     60000
+
+#define TEST_IPV6_ADDR_STR          "2001:db8::1"
+#define TEST_IPV6_ADDR_NETIF_STR    "2001:db8::1%60000"
+
+#define TEST_IPV6_FMT_UDP_EP        {                               \
+                                        .family = AF_INET6,         \
+                                        .addr = {                   \
+                                            .ipv6 = TEST_IPV6_ADDR, \
+                                        },                          \
+                                        .port = 53,                 \
+                                    }
+
+
+#define TEST_URL                    "http://[2001:db8::1]:80/local"
+#define TEST_URL_HOSTPART           "[2001:db8::1]:80"
+#define TEST_URL_LOCALPART          "/local"
+#define TEST_URL_NOLOCAL            "coap://[2001:db8::1]"
+#define TEST_URL_NOLOCAL_HOSTPART   "[2001:db8::1]"
+#define TEST_URL_DNS                "http://test.local/local"
+#define TEST_URL_DNS_HOSTPART       "test.local"
+#define TEST_URL_INVALID            "[2001:db8::1]://local"
+#define TEST_URL_INVALID2           "[2001:db8::1]/local"
+#define TEST_URL_LONG_HOSTPORT      "http://veryveryvery.long.hostname.that." \
+                                    "doesnt.fit.inside.sixtyfour.characters." \
+                                    "of.buffer.space/localpart"
+#define TEST_URL_LONG_URLPATH       "http://shorthostname/very/very/long/ " \
+                                    "path/that/doesnt/fit/inside/sixtyfour/" \
+                                    "chars/of/buffer/space"
+
+#define TEST_STR2EP                 "[2001:db8::1]"
+#define TEST_STR2EP_V4              "10.0.0.1"
+#define TEST_STR2EP_V4_2            "10.0.0.1:53"
+#define TEST_STR2EP_V4_INVALID      "[10.0.0.1]:53"
+#define TEST_STR2EP_INVALID         "[2001:db8:a:b:c:d:e:f:1]"
+#define TEST_STR2EP_INVALID2        "[2001:db8:a:b:c:d:e:f]:66000"
+
+static char addr[SOCK_URLPATH_MAXLEN];
+static char urlpath[SOCK_URLPATH_MAXLEN];
+
+
+static void setup(void)
+{
+    /* Force both arrays to contain nonzero content to detect missing null
+     * terminator */
+    memset(addr, 1, sizeof(addr));
+    memset(urlpath, 1, sizeof(urlpath));
+}
+
+static void test_sock_util_fmt__netif_unset(void)
+{
+    sock_udp_ep_t ep = TEST_IPV6_FMT_UDP_EP;
+    uint16_t port;
+    TEST_ASSERT_EQUAL_INT(strlen(TEST_IPV6_ADDR_STR),
+            sock_udp_ep_fmt(&ep, addr, &port));
+    TEST_ASSERT_EQUAL_INT(ep.port, port);
+    TEST_ASSERT_EQUAL_STRING(TEST_IPV6_ADDR_STR, (char *)addr);
+}
+
+static void test_sock_util_fmt__netif_set(void)
+{
+    sock_udp_ep_t ep = TEST_IPV6_FMT_UDP_EP;
+    uint16_t port;
+    ep.netif = TEST_IPV6_NETIF;
+    TEST_ASSERT_EQUAL_INT(strlen(TEST_IPV6_ADDR_NETIF_STR),
+                sock_udp_ep_fmt(&ep, addr, &port));
+    TEST_ASSERT_EQUAL_STRING(TEST_IPV6_ADDR_NETIF_STR, (char *)addr);
+}
+
+static void test_sock_util_fmt__unsupported(void)
+{
+    sock_udp_ep_t ep = TEST_IPV6_FMT_UDP_EP;
+    uint16_t port;
+    ep.family = AF_UNIX; /* Intentionally chosen for testing an unsupported
+                            protocol */
+    TEST_ASSERT_EQUAL_INT(sock_udp_ep_fmt(&ep, addr, &port), -ENOTSUP);
+    TEST_ASSERT_EQUAL_STRING("", (char *)addr);
+}
+
+static void test_sock_util_urlsplit__host_path(void)
+{
+    TEST_ASSERT_EQUAL_INT(0,
+            sock_urlsplit(TEST_URL, addr, urlpath));
+    TEST_ASSERT_EQUAL_STRING(TEST_URL_HOSTPART, (char*)addr);
+    TEST_ASSERT_EQUAL_STRING(TEST_URL_LOCALPART, (char*)urlpath);
+}
+
+static void test_sock_util_urlsplit__no_path(void)
+{
+    TEST_ASSERT_EQUAL_INT(0,
+            sock_urlsplit(TEST_URL_NOLOCAL, addr, urlpath));
+    TEST_ASSERT_EQUAL_STRING(TEST_URL_NOLOCAL_HOSTPART, (char*)addr);
+    TEST_ASSERT_EQUAL_INT(0, strlen(urlpath));
+}
+
+static void test_sock_util_urlsplit__dnsname(void)
+{
+    TEST_ASSERT_EQUAL_INT(0,
+            sock_urlsplit(TEST_URL_DNS, addr, urlpath));
+    TEST_ASSERT_EQUAL_STRING(TEST_URL_DNS_HOSTPART, (char*)addr);
+    TEST_ASSERT_EQUAL_STRING(TEST_URL_LOCALPART, (char*)urlpath);
+}
+
+static void test_sock_util_urlsplit__invalid_sep(void)
+{
+    TEST_ASSERT_EQUAL_INT(-EINVAL,
+            sock_urlsplit(TEST_URL_INVALID, addr, urlpath));
+}
+
+static void test_sock_util_urlsplit__no_schema(void)
+{
+    TEST_ASSERT_EQUAL_INT(-EINVAL,
+            sock_urlsplit(TEST_URL_INVALID2, addr, urlpath));
+}
+
+static void test_sock_util_urlsplit__hostport_too_long(void)
+{
+    TEST_ASSERT_EQUAL_INT(-EOVERFLOW,
+            sock_urlsplit(TEST_URL_LONG_HOSTPORT, addr, urlpath));
+}
+
+static void test_sock_util_urlsplit__urlpath_too_long(void)
+{
+    TEST_ASSERT_EQUAL_INT(-EOVERFLOW,
+            sock_urlsplit(TEST_URL_LONG_URLPATH, addr, urlpath));
+}
+
+static void test_sock_util_str2ep__ipv6_noport(void)
+{
+    sock_udp_ep_t ep;
+    ep.port = 0;
+    TEST_ASSERT_EQUAL_INT(0, sock_udp_str2ep(&ep, TEST_STR2EP));
+    TEST_ASSERT_EQUAL_INT(0, ep.port);
+    TEST_ASSERT_EQUAL_INT(AF_INET6, ep.family);
+}
+
+static void test_sock_util_str2ep__ipv4_noport(void)
+{
+    sock_udp_ep_t ep;
+    ep.port = 0;
+    TEST_ASSERT_EQUAL_INT(0, sock_udp_str2ep(&ep, TEST_STR2EP_V4));
+    TEST_ASSERT_EQUAL_INT(0, ep.port);
+    TEST_ASSERT_EQUAL_INT(AF_INET, ep.family);
+}
+
+static void test_sock_util_str2ep__ipv4_port(void)
+{
+    sock_udp_ep_t ep;
+    TEST_ASSERT_EQUAL_INT(0, sock_udp_str2ep(&ep, TEST_STR2EP_V4_2));
+    TEST_ASSERT_EQUAL_INT(53, ep.port);
+    TEST_ASSERT_EQUAL_INT(AF_INET, ep.family);
+}
+
+static void test_sock_util_str2ep__ipv4_bracketed(void)
+{
+    sock_udp_ep_t ep;
+    TEST_ASSERT_EQUAL_INT(-EINVAL, sock_udp_str2ep(&ep,
+                                                   TEST_STR2EP_V4_INVALID));
+}
+
+static void test_sock_util_str2ep__invalid_ipv6(void)
+{
+    sock_udp_ep_t ep;
+    TEST_ASSERT_EQUAL_INT(-EINVAL, sock_udp_str2ep(&ep, TEST_STR2EP_INVALID));
+}
+
+static void test_sock_util_str2ep__invalid_port(void)
+{
+    sock_udp_ep_t ep;
+    TEST_ASSERT_EQUAL_INT(-EINVAL, sock_udp_str2ep(&ep, TEST_STR2EP_INVALID2));
+}
+
+Test *tests_sock_util_all(void)
+{
+    EMB_UNIT_TESTFIXTURES(fixtures) {
+        new_TestFixture(test_sock_util_fmt__netif_unset),
+        new_TestFixture(test_sock_util_fmt__netif_set),
+        new_TestFixture(test_sock_util_fmt__unsupported),
+        new_TestFixture(test_sock_util_urlsplit__host_path),
+        new_TestFixture(test_sock_util_urlsplit__no_path),
+        new_TestFixture(test_sock_util_urlsplit__dnsname),
+        new_TestFixture(test_sock_util_urlsplit__invalid_sep),
+        new_TestFixture(test_sock_util_urlsplit__no_schema),
+        new_TestFixture(test_sock_util_urlsplit__hostport_too_long),
+        new_TestFixture(test_sock_util_urlsplit__urlpath_too_long),
+        new_TestFixture(test_sock_util_str2ep__ipv6_noport),
+        new_TestFixture(test_sock_util_str2ep__ipv4_noport),
+        new_TestFixture(test_sock_util_str2ep__ipv4_port),
+        new_TestFixture(test_sock_util_str2ep__ipv4_bracketed),
+        new_TestFixture(test_sock_util_str2ep__invalid_ipv6),
+        new_TestFixture(test_sock_util_str2ep__invalid_port),
+    };
+
+    EMB_UNIT_TESTCALLER(sockutil_tests, setup, NULL, fixtures);
+    return (Test *)&sockutil_tests;
+}
+
+void tests_sock_util(void)
+{
+    TESTS_RUN(tests_sock_util_all());
+}

tests/unittests/tests-sock_util/tests-sock_util.h

+/*
+ * Copyright (C) 2018 Freie Universität Berlin
+ * Copyright (C) 2018 Inria
+ *
+ * This file is subject to the terms and conditions of the GNU Lesser
+ * General Public License v2.1. See the file LICENSE in the top level
+ * directory for more details.
+ */
+
+/**
+ * @addtogroup  unittests
+ * @{
+ *
+ * @file
+ * @brief       Unittests for the sock_util module
+ *
+ */
+#ifndef TESTS_SOCK_UTIL_H
+#define TESTS_SOCK_UTIL_H
+
+#include "embUnit/embUnit.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ *  @brief   The entry point of this test suite.
+ */
+void tests_sockutil(void);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* TESTS_SOCK_UTIL_H */
+/** @} */