Fix identity buffer length check issue #175

zerotier · Feb 27, 2024 · ddff3f9 · ddff3f9
1 parent 403d262
commit ddff3f9
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 29 deletions.
diff --git a/src/Controls.cpp b/src/Controls.cpp
@@ -304,6 +304,12 @@ int zts_id_pair_is_valid(const char* key, unsigned int len)
 int zts_node_get_id_pair(char* key, unsigned int* key_dst_len)
 {
     ACQUIRE_SERVICE(ZTS_ERR_SERVICE);
+    if (key == NULL || key_dst_len == NULL) {
+        return ZTS_ERR_ARG;
+    }
+    if (*key_dst_len != ZT_IDENTITY_STRING_BUFFER_LENGTH) {
+        return ZTS_ERR_ARG;
+    }
     int err = ZTS_ERR_OK;
     if ((err = zts_service->getIdentity(key, key_dst_len)) != ZTS_ERR_OK) {
         return err;

diff --git a/src/NodeService.cpp b/src/NodeService.cpp
@@ -1065,7 +1065,7 @@ void NodeService::sendEventToUser(unsigned int zt_event_code, const void* obj, u
             pr->role = static_cast<zts_peer_role_t>(peer->role);
             pr->path_count = peer->pathCount;
 
-            for (unsigned int j = 0; j < peer->pathCount; j++) {               
+            for (unsigned int j = 0; j < peer->pathCount; j++) {
                 native_ss_to_zts_ss(&(pr->paths[j].address), &(peer->paths[j].address));
                 pr->paths[j].last_tx = peer->paths[j].lastSend;
                 pr->paths[j].last_rx = peer->paths[j].lastReceive;
@@ -1074,7 +1074,7 @@ void NodeService::sendEventToUser(unsigned int zt_event_code, const void* obj, u
 
                 // TODO: ZT_PeerPhysicalPath has the ifname as an internal buffer instead of a pointer
                 // for now this seems like the best solution
-                pr->paths[j].ifname = nullptr; 
+                pr->paths[j].ifname = nullptr;
 
                 pr->paths[j].expired = peer->paths[j].expired;
                 pr->paths[j].preferred = peer->paths[j].preferred;
@@ -1509,8 +1509,8 @@ uint64_t NodeService::getNodeId()
 
 int NodeService::setIdentity(const char* keypair, unsigned int len)
 {
-    if (keypair == NULL || len < ZT_IDENTITY_STRING_BUFFER_LENGTH) {
-        //   return ZTS_ERR_ARG;
+    if (keypair == NULL || len != ZT_IDENTITY_STRING_BUFFER_LENGTH) {
+        return ZTS_ERR_ARG;
     }
     // Double check user-provided keypair
     Identity id;
@@ -1530,7 +1530,7 @@ int NodeService::setIdentity(const char* keypair, unsigned int len)
 
 int NodeService::getIdentity(char* keypair, unsigned int* len)
 {
-    if (keypair == NULL || *len < ZT_IDENTITY_STRING_BUFFER_LENGTH) {
+    if (keypair == NULL || *len != ZT_IDENTITY_STRING_BUFFER_LENGTH) {
         return ZTS_ERR_ARG;
     }
     if (_node) {

diff --git a/test/selftest.c b/test/selftest.c
@@ -1353,28 +1353,6 @@ void test_client_socket_usage(char* ip4, uint16_t port4, char* ip6, uint16_t por
 // Start node                                                                 //
 //----------------------------------------------------------------------------//
 
-int test_api_abuse()
-{
-    /*
-
-    TODO
-
-    DEBUG_INFO("\n\n***\ttest_api_abuse");
-    if (join_network) {
-        for (int i = 0; i < 1024 * 4; i++) {
-            printf("join %d\n", i);
-            zts_net_join(net_id);
-            printf("leave %d\n", i);
-            zts_net_leave(net_id);
-        }
-        zts_node_stop();
-        zts_util_delay(2000);
-        exit(0);
-    }
-    */
-    return 0;
-}
-
 int test_start_node(
     char* path,
     uint64_t net_id,
@@ -1407,7 +1385,12 @@ int test_start_node(
         assert(zts_init_set_event_handler(&on_zts_event) == ZTS_ERR_OK);
     }
     if (use_identity) {
-        // TODO: tomorrow
+        //  Test incorrect values
+        assert(zts_init_from_memory(keypair, 0) == ZTS_ERR_ARG);
+        assert(zts_init_from_memory(keypair, ZTS_ID_STR_BUF_LEN+1) == ZTS_ERR_ARG);
+        assert(zts_init_from_memory(keypair, ZTS_ID_STR_BUF_LEN-1) == ZTS_ERR_ARG);
+        assert(zts_init_from_memory(NULL, ZTS_ID_STR_BUF_LEN) == ZTS_ERR_ARG);
+        // Correct
         assert(zts_init_from_memory(keypair, ZTS_ID_STR_BUF_LEN) == ZTS_ERR_OK);
     }
 
@@ -1427,7 +1410,19 @@ int test_start_node(
     // Test identity handling
     char keypair_i[ZTS_ID_STR_BUF_LEN] = { 0 };
     unsigned int keypair_len = ZTS_ID_STR_BUF_LEN;
+    unsigned int key_dst_len = 0;
+    // Test incorrect values
+    key_dst_len = 0;
+    assert(zts_node_get_id_pair(&keypair_i, NULL) == ZTS_ERR_ARG);
+    key_dst_len = ZTS_ID_STR_BUF_LEN+1;
+    assert(zts_node_get_id_pair(&keypair_i, &key_dst_len) == ZTS_ERR_ARG);
+    key_dst_len = ZTS_ID_STR_BUF_LEN-1;
+    assert(zts_node_get_id_pair(&keypair_i, &key_dst_len) == ZTS_ERR_ARG);
+    key_dst_len = ZTS_ID_STR_BUF_LEN;
+    assert(zts_node_get_id_pair(NULL, ZTS_ID_STR_BUF_LEN) == ZTS_ERR_ARG);
+    // Correct
     assert(zts_node_get_id_pair(keypair_i, &keypair_len) == ZTS_ERR_OK);
+
     DEBUG_INFO("Checking key length");
     assert(keypair_len <= ZTS_ID_STR_BUF_LEN);
     DEBUG_INFO("GET [identity = %s]", keypair_i);
@@ -1892,7 +1887,6 @@ int main(int argc, char** argv)
         test_addr_computation();
         test_roots_handling();
         test_start_sequences();
-        test_api_abuse();
         test_stats();
         // test_sockets();
     }