diff --git a/pkg/wakaama/contrib/lwm2m_client_connection.c b/pkg/wakaama/contrib/lwm2m_client_connection.c index 1d91993de1..613cd166c6 100644 --- a/pkg/wakaama/contrib/lwm2m_client_connection.c +++ b/pkg/wakaama/contrib/lwm2m_client_connection.c @@ -252,7 +252,7 @@ static lwm2m_client_connection_t *_connection_create(uint16_t sec_obj_inst_id, lwm2m_client_connection_t *conn = NULL; char uri[MAX_URI_LENGTH]; size_t uri_len = ARRAY_SIZE(uri); - const char *port; + uint16_t port; bool is_bootstrap; DEBUG("Creating connection\n"); @@ -287,19 +287,19 @@ static lwm2m_client_connection_t *_connection_create(uint16_t sec_obj_inst_id, } /* if no port specified, use the default server or BS-server ports */ - if (!parsed_uri.port) { + if (parsed_uri.port == 0) { if (is_bootstrap) { - port = CONFIG_LWM2M_BSSERVER_PORT; + port = atoi(CONFIG_LWM2M_BSSERVER_PORT); } else { - port = CONFIG_LWM2M_STANDARD_PORT; + port = atoi(CONFIG_LWM2M_STANDARD_PORT); } } else { port = parsed_uri.port; } - DEBUG("[_connection_create] Creating connection to Host: %.*s, Port: %s\n", + DEBUG("[_connection_create] Creating connection to Host: %.*s, Port: %u\n", parsed_uri.ipv6addr_len, parsed_uri.ipv6addr, port); /* allocate new connection */ @@ -313,7 +313,7 @@ static lwm2m_client_connection_t *_connection_create(uint16_t sec_obj_inst_id, /* configure to any IPv6 */ conn->remote.family = AF_INET6; conn->remote.netif = SOCK_ADDR_ANY_NETIF; - conn->remote.port = atoi(port); + conn->remote.port = port; if (!ipv6_addr_from_buf((ipv6_addr_t *)&conn->remote.addr.ipv6, parsed_uri.ipv6addr, parsed_uri.ipv6addr_len)) { diff --git a/sys/include/uri_parser.h b/sys/include/uri_parser.h index 07d5d0e4dc..d4acda4dbc 100644 --- a/sys/include/uri_parser.h +++ b/sys/include/uri_parser.h @@ -67,15 +67,16 @@ typedef struct { */ const char *zoneid; - const char *port; /**< port */ + const char *port_str; /**< port as str */ const char *path; /**< path */ const char *query; /**< query */ + uint16_t port; /**< port as uint16_t */ uint16_t scheme_len; /**< length of @ref scheme */ uint16_t userinfo_len; /**< length of @ref userinfo */ uint16_t host_len; /**< length of @ref host */ uint16_t ipv6addr_len; /**< length of @ref ipv6addr */ uint16_t zoneid_len; /**< length of @ref zoneid */ - uint16_t port_len; /**< length of @ref port */ + uint16_t port_str_len; /**< length of @ref port_str */ uint16_t path_len; /**< length of @ref path */ uint16_t query_len; /**< length of @ref query */ } uri_parser_result_t; diff --git a/sys/net/application_layer/gcoap/dns.c b/sys/net/application_layer/gcoap/dns.c index 3ccb87162a..4788fb97a7 100644 --- a/sys/net/application_layer/gcoap/dns.c +++ b/sys/net/application_layer/gcoap/dns.c @@ -466,7 +466,7 @@ static int _set_remote(const uri_parser_result_t *uri_comp, _uri); return -EHOSTUNREACH; } - if (uri_comp->port == NULL) { + if (uri_comp->port == 0) { if (strncmp(_uri, "coap:", sizeof("coap:") - 1) == 0) { remote->port = CONFIG_GCOAP_PORT; } @@ -475,15 +475,7 @@ static int _set_remote(const uri_parser_result_t *uri_comp, } } else { - char port_str[uri_comp->port_len + 1]; - - strncpy(port_str, uri_comp->port, uri_comp->port_len); - port_str[uri_comp->port_len] = '\0'; - remote->port = atoi(port_str); - if (remote->port == 0U) { - DEBUG("gcoap_dns: invalid port %s\n", port_str); - return -EINVAL; - } + remote->port = uri_comp->port; } return 0; } diff --git a/sys/net/application_layer/gcoap/forward_proxy.c b/sys/net/application_layer/gcoap/forward_proxy.c index 5beb915670..b0fbf70dc5 100644 --- a/sys/net/application_layer/gcoap/forward_proxy.c +++ b/sys/net/application_layer/gcoap/forward_proxy.c @@ -174,16 +174,8 @@ static bool _parse_endpoint(sock_udp_ep_t *remote, } memcpy(&remote->addr.ipv6[0], &addr.u8[0], sizeof(addr.u8)); - if (urip->port_len) { - /* copy port string into scratch for atoi */ - memcpy(scratch, urip->port, urip->port_len); - scratch[urip->port_len] = '\0'; - - remote->port = atoi(scratch); - - if (remote->port == 0) { - return false; - } + if (urip->port != 0) { + remote->port = urip->port; } else { remote->port = COAP_PORT; diff --git a/sys/uri_parser/uri_parser.c b/sys/uri_parser/uri_parser.c index 29b2b8857a..f28b6e0211 100644 --- a/sys/uri_parser/uri_parser.c +++ b/sys/uri_parser/uri_parser.c @@ -19,9 +19,12 @@ */ #include +#include #include "uri_parser.h" +#define MAX_PORT_STR_LEN (5) + #define ENABLE_DEBUG 0 #include "debug.h" @@ -93,6 +96,8 @@ bool _consume_port(uri_parser_result_t *result, const char *ipv6_end, { /* check for port after host part */ const char *port_begin = NULL; + uint16_t port_str_len = 0; + /* repeat until last ':' in authority section */ /* if ipv6 address, check after ipv6 end marker */ const char *p = (ipv6_end ? ipv6_end : result->host); @@ -103,22 +108,46 @@ bool _consume_port(uri_parser_result_t *result, const char *ipv6_end, /* check if match */ if (port_begin && (port_begin[0] == ':')) { - /* port should be at least one character, => + 1 */ - if (port_begin + 1 == authority_end) { - return false; - } - result->port = port_begin + 1; - result->port_len = authority_end - result->port; + port_begin++; - /* ports can't exceed 5 characters */ - if (result->port_len > 5){ - result->port = NULL; - result->port_len = 0; + /* port should be at least one character long */ + if (port_begin == authority_end) { return false; } + port_str_len = authority_end - port_begin; + + /* Verify that the port number is up to 5 (random) chars in size */ + if (port_str_len > MAX_PORT_STR_LEN) { + return false; + } + + /* Verify that all characters of the port are numerical */ + for (unsigned int i = 0; i < port_str_len; ++i) { + if (!((port_begin[i] >= '0') && (port_begin[i] <= '9'))) { + return false; + } + } + + /* Verify that the next character, after the port, is an invalid + * character for the atol function. Preventing it from reading out- + * side of the port section */ + if ((authority_end[0] >= '0') && (authority_end[0] <= '9')) { + return false; + } + + /* Verify that the port is smaller or equal to UINT16_MAX. */ + uint32_t port = atol(port_begin); + if (port > UINT16_MAX) { + return false; + } + + result->port = (uint16_t)port; + result->port_str = port_begin; + result->port_str_len = port_str_len; + /* cut host part before port and ':' */ - result->host_len -= result->port_len + 1; + result->host_len -= result->port_str_len + 1; } return true; @@ -300,10 +329,16 @@ int uri_parser_process(uri_parser_result_t *result, const char *uri, memset(result, 0, sizeof(*result)); if (uri_parser_is_absolute(uri, uri_len)) { - return _parse_absolute(result, uri, uri + uri_len); + if (_parse_absolute(result, uri, uri + uri_len) != 0) { + memset(result, 0, sizeof(*result)); + return -1; + } } else { - return _parse_relative(result, uri, uri + uri_len); + if (_parse_relative(result, uri, uri + uri_len) != 0) { + memset(result, 0, sizeof(*result)); + return -1; + } } return 0; diff --git a/tests/unittests/tests-uri_parser/tests-uri_parser.c b/tests/unittests/tests-uri_parser/tests-uri_parser.c index 77c12bab98..77e58e7a7b 100644 --- a/tests/unittests/tests-uri_parser/tests-uri_parser.c +++ b/tests/unittests/tests-uri_parser/tests-uri_parser.c @@ -21,12 +21,23 @@ #include "unittests-constants.h" #include "tests-uri_parser.h" -#define VEC(u, f, s, us, h, v6a, z, po, pa, q, e) \ +#define VEC(u, f, s, us, h, v6a, z, po_str, po, pa, q, e) \ { .uri = u, .full_uri = f, .scheme = s, .userinfo = us, .host = h, \ - .ipv6addr = v6a, .zoneid = z, .port = po, .path = pa, \ - .query = q, .expected = e} + .ipv6addr = v6a, .zoneid = z, .port_str = po_str, .port = po, \ + .path = pa, .query = q, .expected = e } -#define VEC_CHECK(comp, i, vec_msg) \ +#define VEC_CHECK_INT(comp, i, vec_msg) \ + do { \ + vec_msg[0] = '\0'; \ + stdimpl_strcat(vec_msg, "Unexpected " # comp " member "); \ + stdimpl_lltoa(validate_uris[i].comp, &vec_msg[strlen(vec_msg)], 10);\ + stdimpl_strcat(vec_msg, " for \""); \ + stdimpl_strcat(vec_msg, validate_uris[i].uri); \ + stdimpl_strcat(vec_msg, "\""); \ + TEST_ASSERT_MESSAGE(validate_uris[i].comp == ures.comp, vec_msg); \ + } while (0) + +#define VEC_CHECK_STR(comp, i, vec_msg) \ do { \ if (ures.comp == NULL) { \ TEST_ASSERT(validate_uris[i].comp[0] == '\0'); \ @@ -58,7 +69,8 @@ typedef struct { char host[24]; char ipv6addr[16]; char zoneid[8]; - char port[32]; + char port_str[32]; + uint16_t port; char path[48]; char query[32]; int expected; @@ -66,7 +78,7 @@ typedef struct { /* VEC(uri_to_parse, - scheme, userinfo, host, port, + scheme, userinfo, host, port_str, port, path, query, expected return value) */ static const validate_t validate_uris[] = { @@ -84,8 +96,10 @@ static const validate_t validate_uris[] = { "fe80:db8::1", /* parsed zoneid */ "tap0", - /* parsed port */ + /* parsed port_str */ "5683", + /* parsed port */ + 5683, /* parsed path */ "/.well-known/core", /* parsed query */ @@ -100,6 +114,7 @@ static const validate_t validate_uris[] = { "fe80:db8::1", "", "5683", + 5683, "/.well-known/core", "v=1", -1), @@ -111,6 +126,7 @@ static const validate_t validate_uris[] = { "fe80::1", "", "", + 0, "/foo%20bar", "", 0), @@ -122,9 +138,58 @@ static const validate_t validate_uris[] = { "", "", "", + 0, "/.well-known/core", "v=1", 0), + VEC("coap://R@[2001:db8::1]:5/v=1", + true, + "coap", + "R", + "[2001:db8::1]", + "2001:db8::1", + "", + "5", + 5, + "/v=1", + "", + 0), + VEC("coap://R@[2001:db8::1]:5/:v=1", + true, + "coap", + "R", + "[2001:db8::1]", + "2001:db8::1", + "", + "5", + 5, + "/:v=1", + "", + 0), + VEC("cap://R@[2001:db8::1]:5/?v=1", + true, + "cap", + "R", + "[2001:db8::1]", + "2001:db8::1", + "", + "5", + 5, + "/", + "v=1", + 0), + VEC("oap://Y2001:db8::1]:5/av=1", + true, + "oap", + "", + "Y2001:db8::1]", + "", + "", + "5", + 5, + "/av=1", + "", + 0), VEC("coap://R@[2001:db8::1]:5own/v=1", true, "coap", @@ -133,9 +198,10 @@ static const validate_t validate_uris[] = { "2001:db8::1", "", "5own", + 5, "/v=1", "", - 0), + -1), VEC("coap://R@[2001:db8::1]:5own/:v=1", true, "coap", @@ -144,9 +210,10 @@ static const validate_t validate_uris[] = { "2001:db8::1", "", "5own", + 5, "/:v=1", "", - 0), + -1), VEC("cap://R@[2001:db8::1]:5own/?v=1", true, "cap", @@ -155,9 +222,10 @@ static const validate_t validate_uris[] = { "2001:db8::1", "", "5own", + 5, "/", "v=1", - 0), + -1), VEC("oap://Y2001:db8::1]:5own/av=1", true, "oap", @@ -166,9 +234,10 @@ static const validate_t validate_uris[] = { "", "", "5own", + 5, "/av=1", "", - 0), + -1), VEC("//Rb[ʰ00J:d/5v=0", false, "", @@ -177,6 +246,7 @@ static const validate_t validate_uris[] = { "", "", "", + 0, "//Rb[ʰ00J:d/5v=0", "", 0), @@ -188,6 +258,7 @@ static const validate_t validate_uris[] = { "", "", "", + 0, "", "", -1), @@ -199,6 +270,7 @@ static const validate_t validate_uris[] = { "", "", "", + 0, "/R@[2008::1]:5own//R@[2008::1]:5own/", "v=1", 0), @@ -210,6 +282,7 @@ static const validate_t validate_uris[] = { "", "", "", + 0, "/RZ[2001[8:01[8::1]:5o:1]:5oTMv=1", "", 0), @@ -221,6 +294,7 @@ static const validate_t validate_uris[] = { "", "", "", + 0, "////////////////7///v=1", "", 0), @@ -232,6 +306,7 @@ static const validate_t validate_uris[] = { "", "", "", + 0, "coa[:////[2001:db5ow:5own/Ov=1", "", 0), @@ -243,6 +318,7 @@ static const validate_t validate_uris[] = { "", "", "", + 0, "+1-816-555-1212", "", 0), @@ -254,6 +330,7 @@ static const validate_t validate_uris[] = { "", "", "", + 0, "+15105550101,+15105550102", "body=hello%20there", 0), @@ -265,6 +342,7 @@ static const validate_t validate_uris[] = { "", "", "", + 0, "a", "", 0), @@ -276,6 +354,7 @@ static const validate_t validate_uris[] = { "", "", "", + 0, "test@example.com", "", 0), @@ -287,6 +366,7 @@ static const validate_t validate_uris[] = { "", "", "", + 0, "/rfc/rfc1808.txt", "", 0), @@ -298,6 +378,7 @@ static const validate_t validate_uris[] = { "", "", "", + 0, "/rfc/rfc2396.txt", "", 0), @@ -309,6 +390,7 @@ static const validate_t validate_uris[] = { "2001:db8::7", "", "", + 0, "/c=GB", "objectClass?one", 0), @@ -320,6 +402,7 @@ static const validate_t validate_uris[] = { "", "", "", + 0, "John.Doe@example.com", "", 0), @@ -331,6 +414,7 @@ static const validate_t validate_uris[] = { "", "", "", + 0, "comp.infosystems.www.servers.unix", "", 0), @@ -342,6 +426,7 @@ static const validate_t validate_uris[] = { "", "", "", + 0, "+1-816-555-1212", "", 0), @@ -353,6 +438,7 @@ static const validate_t validate_uris[] = { "", "", "80", + 80, "/", "", 0), @@ -364,6 +450,7 @@ static const validate_t validate_uris[] = { "", "", "", + 0, "oasis:names:specification:docbook:dtd:xml:4.1.2", "", 0), @@ -375,6 +462,7 @@ static const validate_t validate_uris[] = { "", "", "", + 0, "", "", -1), @@ -386,6 +474,7 @@ static const validate_t validate_uris[] = { "", "", "", + 0, "/", "", 0), @@ -397,6 +486,7 @@ static const validate_t validate_uris[] = { "", "", "", + 0, "./this:that", "", 0), @@ -408,6 +498,7 @@ static const validate_t validate_uris[] = { "", "", "", + 0, "", "", 0), @@ -419,6 +510,7 @@ static const validate_t validate_uris[] = { "", "", "", + 0, "", "", 0), @@ -435,14 +527,40 @@ static const validate_t validate_uris[] = { "", /* parsed zoneid */ "", - /* parsed port */ + /* parsed port_str */ "", + /* parsed port */ + 0, /* parsed path */ "", /* parsed query */ "", /* expected return value */ -1), + VEC("coap://example.com: 12/foo", + true, + "", + "", + "example.com", + "", + "", + "", + 0, + "", + "", + -1), + VEC("coap://example.com:0x12/foo", + true, + "", + "", + "example.com", + "", + "", + "", + 0, + "", + "", + -1), }; static char _failure_msg[VEC_MSG_LEN]; @@ -456,14 +574,15 @@ static void test_uri_parser__validate(void) uri_parser_is_absolute_string(validate_uris[i].uri)); TEST_ASSERT_EQUAL_INT(validate_uris[i].expected, res); if (res == 0) { - VEC_CHECK(scheme, i, _failure_msg); - VEC_CHECK(userinfo, i, _failure_msg); - VEC_CHECK(host, i, _failure_msg); - VEC_CHECK(ipv6addr, i, _failure_msg); - VEC_CHECK(zoneid, i, _failure_msg); - VEC_CHECK(port, i, _failure_msg); - VEC_CHECK(path, i, _failure_msg); - VEC_CHECK(query, i, _failure_msg); + VEC_CHECK_STR(scheme, i, _failure_msg); + VEC_CHECK_STR(userinfo, i, _failure_msg); + VEC_CHECK_STR(host, i, _failure_msg); + VEC_CHECK_STR(ipv6addr, i, _failure_msg); + VEC_CHECK_STR(zoneid, i, _failure_msg); + VEC_CHECK_STR(port_str, i, _failure_msg); + VEC_CHECK_INT(port, i, _failure_msg); + VEC_CHECK_STR(path, i, _failure_msg); + VEC_CHECK_STR(query, i, _failure_msg); } } } @@ -480,14 +599,15 @@ static void test_uri_parser__unterminated_string(void) int res = uri_parser_process(&ures, uri, strlen(validate_uris[0].uri)); TEST_ASSERT_EQUAL_INT(0, res); - VEC_CHECK(scheme, 0, _failure_msg); - VEC_CHECK(userinfo, 0, _failure_msg); - VEC_CHECK(host, 0, _failure_msg); - VEC_CHECK(ipv6addr, 0, _failure_msg); - VEC_CHECK(zoneid, 0, _failure_msg); - VEC_CHECK(port, 0, _failure_msg); - VEC_CHECK(path, 0, _failure_msg); - VEC_CHECK(query, 0, _failure_msg); + VEC_CHECK_STR(scheme, 0, _failure_msg); + VEC_CHECK_STR(userinfo, 0, _failure_msg); + VEC_CHECK_STR(host, 0, _failure_msg); + VEC_CHECK_STR(ipv6addr, 0, _failure_msg); + VEC_CHECK_STR(zoneid, 0, _failure_msg); + VEC_CHECK_STR(port_str, 0, _failure_msg); + VEC_CHECK_INT(port, 0, _failure_msg); + VEC_CHECK_STR(path, 0, _failure_msg); + VEC_CHECK_STR(query, 0, _failure_msg); } Test *tests_uri_parser_tests(void)