Browse Source

Additional refactoring of ns_turn_allocation.* to address security scanner concerns (#1514)

You can see the list here:
https://github.com/coturn/coturn/security/code-scanning

In this case, i'm attempting to address:

ns_turn_allocation.c:725 -- Dereferencing NULL pointer. 'ub->bufs'
contains the same NULL value as 'realloc()' did.
ns_turn_allocation.c:724 -- 'realloc' might return null pointer:
assigning null pointer to 'ub->bufs', which is passed as an argument to
'realloc', will cause the original memory block to be leaked.
ns_turn_allocation.c:604 -- Dereferencing NULL pointer. 'a->tcs.elems'
contains the same NULL value as 'realloc()' did.
    ns_turn_allocation.c:582 -- Dereferencing NULL pointer 'tc'.
ns_turn_allocation.c:603 -- 'realloc' might return null pointer:
assigning null pointer to 'a->tcs.elems', which is passed as an argument
to 'realloc', will cause the original memory block to be leaked.
    ns_turn_allocation.c:525 -- Using uninitialized memory '*chi'.
    ns_turn_allocation.c:229 -- Using uninitialized memory '*slot'.

---------

Co-authored-by: Pavel Punsky <[email protected]>
Michael Jones 1 year ago
parent
commit
af4c44a818
3 changed files with 300 additions and 301 deletions
  1. 6 6
      src/client++/TurnMsgLib.h
  2. 284 287
      src/server/ns_turn_allocation.c
  3. 10 8
      src/server/ns_turn_allocation.h

+ 6 - 6
src/client++/TurnMsgLib.h

@@ -731,7 +731,7 @@ protected:
  */
 class StunMsgRequest : public StunMsg {
 public:
-  StunMsgRequest(uint16_t method) : _method(method) {};
+  StunMsgRequest(uint16_t method) : _method(method){};
   StunMsgRequest(uint8_t *buffer, size_t total_sz, size_t sz, bool constructed)
       : StunMsg(buffer, total_sz, sz, constructed), _method(0) {
 
@@ -804,11 +804,11 @@ private:
  */
 class StunMsgResponse : public StunMsg {
 public:
-  StunMsgResponse(uint16_t method, stun_tid &tid) : _method(method), _err(0), _reason(""), _tid(tid) {};
+  StunMsgResponse(uint16_t method, stun_tid &tid) : _method(method), _err(0), _reason(""), _tid(tid){};
   StunMsgResponse(uint16_t method, int error_code, std::string reason, stun_tid &tid)
-      : _method(method), _err(error_code), _reason(reason), _tid(tid) {
+      : _method(method), _err(error_code), _reason(reason), _tid(tid){
 
-        };
+                                                            };
   StunMsgResponse(uint8_t *buffer, size_t total_sz, size_t sz, bool constructed)
       : StunMsg(buffer, total_sz, sz, constructed), _method(0), _err(0), _reason("") {
 
@@ -960,7 +960,7 @@ private:
  */
 class StunMsgIndication : public StunMsg {
 public:
-  StunMsgIndication(uint16_t method) : _method(method) {};
+  StunMsgIndication(uint16_t method) : _method(method){};
   StunMsgIndication(uint8_t *buffer, size_t total_sz, size_t sz, bool constructed)
       : StunMsg(buffer, total_sz, sz, constructed), _method(0) {
 
@@ -1005,7 +1005,7 @@ private:
  */
 class StunMsgChannel : public StunMsg {
 public:
-  StunMsgChannel(uint16_t cn, int length) : _cn(cn), _len(length) {};
+  StunMsgChannel(uint16_t cn, int length) : _cn(cn), _len(length){};
   StunMsgChannel(uint8_t *buffer, size_t total_sz, size_t sz, bool constructed)
       : StunMsg(buffer, total_sz, sz, constructed), _cn(0) {
 

+ 284 - 287
src/server/ns_turn_allocation.c

@@ -54,42 +54,38 @@ void init_allocation(void *owner, allocation *a, ur_map *tcp_connections) {
 }
 
 void clear_allocation(allocation *a, SOCKET_TYPE socket_type) {
-  if (a) {
+  if (!a) {
+    return;
+  }
 
-    if (a->is_valid) {
-      turn_report_allocation_delete(a, socket_type);
-    }
+  if (a->is_valid) {
+    turn_report_allocation_delete(a, socket_type);
+  }
 
-    if (a->tcs.elems) {
-      size_t i;
-      size_t sz = a->tcs.sz;
-      for (i = 0; i < sz; ++i) {
-        tcp_connection *tc = a->tcs.elems[i];
-        if (tc) {
-          delete_tcp_connection(tc);
-          a->tcs.elems[i] = NULL;
-        }
-      }
-      free(a->tcs.elems);
-      a->tcs.elems = NULL;
-    }
-    a->tcs.sz = 0;
-
-    {
-      int i;
-      for (i = 0; i < ALLOC_PROTOCOLS_NUMBER; ++i) {
-        clear_ioa_socket_session_if(a->relay_sessions[i].s, a->owner);
-        clear_relay_endpoint_session_data(&(a->relay_sessions[i]));
-        IOA_EVENT_DEL(a->relay_sessions[i].lifetime_ev);
+  if (a->tcs.elems) {
+    for (size_t i = 0; i < a->tcs.sz; ++i) {
+      tcp_connection *tc = a->tcs.elems[i];
+      if (tc) {
+        delete_tcp_connection(tc);
+        a->tcs.elems[i] = NULL;
       }
     }
+    free(a->tcs.elems);
+    a->tcs.elems = NULL;
+  }
+  a->tcs.sz = 0;
 
-    /* The order is important here: */
-    free_turn_permission_hashtable(&(a->addr_to_perm));
-    ch_map_clean(&(a->chns));
-
-    a->is_valid = 0;
+  for (size_t i = 0; i < ALLOC_PROTOCOLS_NUMBER; ++i) {
+    clear_ioa_socket_session_if(a->relay_sessions[i].s, a->owner);
+    clear_relay_endpoint_session_data(&(a->relay_sessions[i]));
+    IOA_EVENT_DEL(a->relay_sessions[i].lifetime_ev);
   }
+
+  /* The order is important here: */
+  free_turn_permission_hashtable(&(a->addr_to_perm));
+  ch_map_clean(&(a->chns));
+
+  a->is_valid = 0;
 }
 
 relay_endpoint_session *get_relay_session(allocation *a, int family) {
@@ -120,27 +116,27 @@ ioa_socket_handle get_relay_socket(allocation *a, int family) {
 }
 
 void set_allocation_family_invalid(allocation *a, int family) {
-  if (a) {
-    size_t index = ALLOC_INDEX(family);
-    if (a->relay_sessions[index].s) {
-      if (a->tcs.elems) {
-        size_t i;
-        size_t sz = a->tcs.sz;
-        for (i = 0; i < sz; ++i) {
-          tcp_connection *tc = a->tcs.elems[i];
-          if (tc) {
-            if (tc->peer_s && (get_ioa_socket_address_family(tc->peer_s) == family)) {
-              delete_tcp_connection(tc);
-              a->tcs.elems[i] = NULL;
-            }
+  if (!a) {
+    return;
+  }
+
+  const size_t index = ALLOC_INDEX(family);
+  if (a->relay_sessions[index].s) {
+    if (a->tcs.elems) {
+      for (size_t i = 0; i < a->tcs.sz; ++i) {
+        tcp_connection *tc = a->tcs.elems[i];
+        if (tc) {
+          if (tc->peer_s && (get_ioa_socket_address_family(tc->peer_s) == family)) {
+            delete_tcp_connection(tc);
+            a->tcs.elems[i] = NULL;
           }
         }
       }
-
-      clear_ioa_socket_session_if(a->relay_sessions[index].s, a->owner);
-      clear_relay_endpoint_session_data(&(a->relay_sessions[index]));
-      IOA_EVENT_DEL(a->relay_sessions[index].lifetime_ev);
     }
+
+    clear_ioa_socket_session_if(a->relay_sessions[index].s, a->owner);
+    clear_relay_endpoint_session_data(&(a->relay_sessions[index]));
+    IOA_EVENT_DEL(a->relay_sessions[index].lifetime_ev);
   }
 }
 
@@ -152,15 +148,14 @@ void set_allocation_lifetime_ev(allocation *a, turn_time_t exp_time, ioa_timer_h
   }
 }
 
-int is_allocation_valid(const allocation *a) {
+bool is_allocation_valid(const allocation *a) {
   if (a) {
     return a->is_valid;
-  } else {
-    return 0;
   }
+  return false;
 }
 
-void set_allocation_valid(allocation *a, int value) {
+void set_allocation_valid(allocation *a, bool value) {
   if (a) {
     a->is_valid = value;
   }
@@ -178,23 +173,24 @@ turn_permission_info *allocation_get_permission(allocation *a, const ioa_addr *a
 static bool delete_channel_info_from_allocation_map(ur_map_key_type key, ur_map_value_type value);
 
 void turn_permission_clean(turn_permission_info *tinfo) {
-  if (tinfo && tinfo->allocated) {
-
-    if (tinfo->verbose) {
-      char s[257] = "\0";
-      addr_to_string(&(tinfo->addr), (uint8_t *)s);
-      TURN_LOG_FUNC(TURN_LOG_LEVEL_INFO, "session %018llu: peer %s deleted\n", tinfo->session_id, s);
-    }
+  if (!tinfo || !tinfo->allocated) {
+    return;
+  }
 
-    if (!(tinfo->lifetime_ev)) {
-      TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "!!! %s: strange (1) permission to be cleaned\n", __FUNCTION__);
-    }
+  if (tinfo->verbose) {
+    char s[257] = "\0";
+    addr_to_string(&(tinfo->addr), (uint8_t *)s);
+    TURN_LOG_FUNC(TURN_LOG_LEVEL_INFO, "session %018llu: peer %s deleted\n", tinfo->session_id, s);
+  }
 
-    IOA_EVENT_DEL(tinfo->lifetime_ev);
-    lm_map_foreach(&(tinfo->chns), (foreachcb_type)delete_channel_info_from_allocation_map);
-    lm_map_clean(&(tinfo->chns));
-    memset(tinfo, 0, sizeof(turn_permission_info));
+  if (!(tinfo->lifetime_ev)) {
+    TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "!!! %s: strange (1) permission to be cleaned\n", __FUNCTION__);
   }
+
+  IOA_EVENT_DEL(tinfo->lifetime_ev);
+  lm_map_foreach(&(tinfo->chns), (foreachcb_type)delete_channel_info_from_allocation_map);
+  lm_map_clean(&(tinfo->chns));
+  memset(tinfo, 0, sizeof(turn_permission_info));
 }
 
 static void init_turn_permission_hashtable(turn_permission_hashtable *map) {
@@ -204,39 +200,36 @@ static void init_turn_permission_hashtable(turn_permission_hashtable *map) {
 }
 
 static void free_turn_permission_hashtable(turn_permission_hashtable *map) {
-  if (map) {
+  if (!map) {
+    return;
+  }
 
-    size_t i;
-    for (i = 0; i < TURN_PERMISSION_HASHTABLE_SIZE; ++i) {
+  for (size_t i = 0; i < TURN_PERMISSION_HASHTABLE_SIZE; ++i) {
 
-      turn_permission_array *parray = &(map->table[i]);
+    turn_permission_array *parray = &(map->table[i]);
 
-      {
-        size_t j;
-        for (j = 0; j < TURN_PERMISSION_ARRAY_SIZE; ++j) {
-          turn_permission_slot *slot = &(parray->main_slots[j]);
-          if (slot->info.allocated) {
-            turn_permission_clean(&(slot->info));
-          }
-        }
+    for (size_t j = 0; j < TURN_PERMISSION_ARRAY_SIZE; ++j) {
+      turn_permission_slot *slot = &(parray->main_slots[j]);
+      if (slot->info.allocated) {
+        turn_permission_clean(&(slot->info));
       }
+    }
 
-      if (parray->extra_slots) {
-        size_t j;
-        for (j = 0; j < parray->extra_sz; ++j) {
-          turn_permission_slot *slot = parray->extra_slots[j];
-          if (slot) {
-            if (slot->info.allocated) {
-              turn_permission_clean(&(slot->info));
-            }
-            free(slot);
+    if (parray->extra_slots) {
+      for (size_t j = 0; j < parray->extra_sz; ++j) {
+        turn_permission_slot *slot = parray->extra_slots[j];
+        if (slot) {
+          if (slot->info.allocated) {
+            turn_permission_clean(&(slot->info));
           }
+          free(slot);
+          parray->extra_slots[j] = NULL;
         }
-        free(parray->extra_slots);
-        parray->extra_slots = NULL;
       }
-      parray->extra_sz = 0;
+      free(parray->extra_slots);
+      parray->extra_slots = NULL;
     }
+    parray->extra_sz = 0;
   }
 }
 
@@ -248,21 +241,15 @@ static turn_permission_info *get_from_turn_permission_hashtable(turn_permission_
   uint32_t index = addr_hash_no_port(addr) & (TURN_PERMISSION_HASHTABLE_SIZE - 1);
   turn_permission_array *parray = &(map->table[index]);
 
-  {
-    size_t i;
-    for (i = 0; i < TURN_PERMISSION_ARRAY_SIZE; ++i) {
-      turn_permission_slot *slot = &(parray->main_slots[i]);
-      if (slot->info.allocated && addr_eq_no_port(&(slot->info.addr), addr)) {
-        return &(slot->info);
-      }
+  for (size_t i = 0; i < TURN_PERMISSION_ARRAY_SIZE; ++i) {
+    turn_permission_slot *slot = &(parray->main_slots[i]);
+    if (slot->info.allocated && addr_eq_no_port(&(slot->info.addr), addr)) {
+      return &(slot->info);
     }
   }
 
   if (parray->extra_slots) {
-
-    size_t i;
-    size_t sz = parray->extra_sz;
-    for (i = 0; i < sz; ++i) {
+    for (size_t i = 0; i < parray->extra_sz; ++i) {
       turn_permission_slot *slot = parray->extra_slots[i];
       if (slot->info.allocated && addr_eq_no_port(&(slot->info.addr), addr)) {
         return &(slot->info);
@@ -301,38 +288,48 @@ static bool delete_channel_info_from_allocation_map(ur_map_key_type key, ur_map_
 }
 
 void turn_channel_delete(ch_info *chn) {
-  if (chn) {
-    int port = addr_get_port(&(chn->peer_addr));
-    if (port < 1) {
-      char s[129];
-      addr_to_string(&(chn->peer_addr), (uint8_t *)s);
-      TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "!!! %s: strange (1) channel to be cleaned: port is empty: %s\n",
-                    __FUNCTION__, s);
-    }
-    {
-      turn_permission_info *tinfo = (turn_permission_info *)chn->owner;
-      if (tinfo) {
-        lm_map_del(&(tinfo->chns), (ur_map_key_type)port, NULL);
-      } else {
-        TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "!!! %s: strange (2) channel to be cleaned: permission is empty\n",
-                      __FUNCTION__);
-      }
-    }
-    delete_channel_info_from_allocation_map((ur_map_key_type)port, (ur_map_value_type)chn);
+  if (!chn) {
+    return;
+  }
+
+  int port = addr_get_port(&(chn->peer_addr));
+  if (port < 1) {
+    char s[129];
+    addr_to_string(&(chn->peer_addr), (uint8_t *)s);
+    TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "!!! %s: strange (1) channel to be cleaned: port is empty: %s\n", __FUNCTION__,
+                  s);
+  }
+
+  turn_permission_info *tinfo = (turn_permission_info *)chn->owner;
+  if (tinfo) {
+    lm_map_del(&(tinfo->chns), (ur_map_key_type)port, NULL);
+  } else {
+    TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "!!! %s: strange (2) channel to be cleaned: permission is empty\n",
+                  __FUNCTION__);
   }
+
+  delete_channel_info_from_allocation_map((ur_map_key_type)port, (ur_map_value_type)chn);
 }
 
 ch_info *allocation_get_new_ch_info(allocation *a, uint16_t chnum, ioa_addr *peer_addr) {
+  if (!a) {
+    return NULL;
+  }
 
   turn_permission_info *tinfo = get_from_turn_permission_hashtable(&(a->addr_to_perm), peer_addr);
-
   if (!tinfo) {
     tinfo = allocation_add_permission(a, peer_addr);
+    if (!tinfo) {
+      return NULL;
+    }
   }
 
   ch_info *chn = ch_map_get(&(a->chns), chnum, 1);
+  if (!chn) {
+    return NULL;
+  }
 
-  chn->allocated = 1;
+  chn->allocated = true;
   chn->chnum = chnum;
   chn->port = addr_get_port(peer_addr);
   addr_cpy(&(chn->peer_addr), peer_addr);
@@ -384,20 +381,34 @@ ch_info *get_turn_channel(turn_permission_info *tinfo, ioa_addr *addr) {
 turn_permission_hashtable *allocation_get_turn_permission_hashtable(allocation *a) { return &(a->addr_to_perm); }
 
 turn_permission_info *allocation_add_permission(allocation *a, const ioa_addr *addr) {
-  if (a && addr) {
+  if (!a || !addr) {
+    return NULL;
+  }
+
+  turn_permission_hashtable *map = &(a->addr_to_perm);
+  uint32_t hash = addr_hash_no_port(addr);
+  size_t fds = (size_t)(hash & (TURN_PERMISSION_HASHTABLE_SIZE - 1));
 
-    turn_permission_hashtable *map = &(a->addr_to_perm);
-    uint32_t hash = addr_hash_no_port(addr);
-    size_t fds = (size_t)(hash & (TURN_PERMISSION_HASHTABLE_SIZE - 1));
+  turn_permission_array *parray = &(map->table[fds]);
 
-    turn_permission_array *parray = &(map->table[fds]);
+  turn_permission_slot *slot = NULL;
+
+  for (size_t i = 0; i < TURN_PERMISSION_ARRAY_SIZE; ++i) {
+    slot = &(parray->main_slots[i]);
+    if (!(slot->info.allocated)) {
+      break;
+    } else {
+      slot = NULL;
+    }
+  }
 
-    turn_permission_slot *slot = NULL;
+  if (!slot) {
+    size_t old_sz = parray->extra_sz;
+    turn_permission_slot **slots = parray->extra_slots;
 
-    {
-      size_t i;
-      for (i = 0; i < TURN_PERMISSION_ARRAY_SIZE; ++i) {
-        slot = &(parray->main_slots[i]);
+    if (slots) {
+      for (size_t i = 0; i < old_sz; ++i) {
+        slot = slots[i];
         if (!(slot->info.allocated)) {
           break;
         } else {
@@ -407,168 +418,151 @@ turn_permission_info *allocation_add_permission(allocation *a, const ioa_addr *a
     }
 
     if (!slot) {
-
-      size_t old_sz = parray->extra_sz;
-
-      turn_permission_slot **slots = parray->extra_slots;
-
-      if (slots) {
-        size_t i;
-        for (i = 0; i < old_sz; ++i) {
-          slot = slots[i];
-          if (!(slot->info.allocated)) {
-            break;
-          } else {
-            slot = NULL;
-          }
-        }
+      size_t old_sz_mem = old_sz * sizeof(turn_permission_slot *);
+      turn_permission_slot **new_slots =
+          (turn_permission_slot **)realloc(parray->extra_slots, old_sz_mem + sizeof(turn_permission_slot *));
+      if (!new_slots) {
+        return NULL;
       }
-
+      parray->extra_slots = new_slots;
+      slots = parray->extra_slots;
+      parray->extra_sz = old_sz + 1;
+      slot = (turn_permission_slot *)malloc(sizeof(turn_permission_slot));
       if (!slot) {
-        size_t old_sz_mem = old_sz * sizeof(turn_permission_slot *);
-        parray->extra_slots =
-            (turn_permission_slot **)realloc(parray->extra_slots, old_sz_mem + sizeof(turn_permission_slot *));
-        slots = parray->extra_slots;
-        parray->extra_sz = old_sz + 1;
-        slots[old_sz] = (turn_permission_slot *)malloc(sizeof(turn_permission_slot));
-        slot = slots[old_sz];
+        return NULL;
       }
+      slots[old_sz] = slot;
     }
+  }
 
-    memset(slot, 0, sizeof(turn_permission_slot));
-    slot->info.allocated = 1;
-    turn_permission_info *elem = &(slot->info);
-    addr_cpy(&(elem->addr), addr);
-    elem->owner = a;
+  memset(slot, 0, sizeof(turn_permission_slot));
+  slot->info.allocated = true;
+  turn_permission_info *elem = &(slot->info);
+  addr_cpy(&(elem->addr), addr);
+  elem->owner = a;
 
-    return elem;
-  } else {
-    return NULL;
-  }
+  return elem;
 }
 
 ch_info *ch_map_get(ch_map *const map, const uint16_t chnum, const int new_chn) {
-  if (map) {
-    const size_t index = (size_t)(chnum & (CH_MAP_HASH_SIZE - 1));
-    ch_map_array *const a = &(map->table[index]);
+  if (!map) {
+    return NULL;
+  }
 
-    for (size_t i = 0; i < CH_MAP_ARRAY_SIZE; ++i) {
-      ch_info *const chi = &(a->main_chns[i]);
-      if (chi->allocated) {
-        if (!new_chn && (chi->chnum == chnum)) {
-          return chi;
-        }
-      } else if (new_chn) {
+  const size_t index = (size_t)(chnum & (CH_MAP_HASH_SIZE - 1));
+  ch_map_array *const a = &(map->table[index]);
+
+  for (size_t i = 0; i < CH_MAP_ARRAY_SIZE; ++i) {
+    ch_info *const chi = &(a->main_chns[i]);
+    if (chi->allocated) {
+      if (!new_chn && (chi->chnum == chnum)) {
         return chi;
       }
+    } else if (new_chn) {
+      return chi;
     }
+  }
 
-    const size_t old_sz = a->extra_sz;
-    if (old_sz && a->extra_chns) {
-      for (size_t i = 0; i < old_sz; ++i) {
-        ch_info *const chi = a->extra_chns[i];
-        if (chi) {
-          if (chi->allocated) {
-            if (!new_chn && (chi->chnum == chnum)) {
-              return chi;
-            }
-          } else if (new_chn) {
+  const size_t old_sz = a->extra_sz;
+  if (old_sz && a->extra_chns) {
+    for (size_t i = 0; i < old_sz; ++i) {
+      ch_info *const chi = a->extra_chns[i];
+      if (chi) {
+        if (chi->allocated) {
+          if (!new_chn && (chi->chnum == chnum)) {
             return chi;
           }
+        } else if (new_chn) {
+          return chi;
         }
       }
     }
+  }
 
-    if (new_chn) {
-      const size_t old_sz_mem = old_sz * sizeof(ch_info *);
-      ch_info **const pTmp = (ch_info **)realloc(a->extra_chns, old_sz_mem + sizeof(ch_info *));
-      if (!pTmp) {
-        return NULL;
-      }
-      a->extra_chns = pTmp;
-      a->extra_chns[old_sz] = (ch_info *)calloc(1, sizeof(ch_info));
-      if (!a->extra_chns[old_sz]) {
-        // if the realloc succeeds, but the calloc fails, we don't attempt to shrink the realloc back down
-        // by not recording the change to the size, we allow the next call to this function to realloc the
-        // block to presumably the same size it already is, which should be fine and not result in any leaks.
-        return NULL;
-      }
-      a->extra_sz += 1;
-
-      return a->extra_chns[old_sz];
+  if (new_chn) {
+    const size_t old_sz_mem = old_sz * sizeof(ch_info *);
+    ch_info **const pTmp = (ch_info **)realloc(a->extra_chns, old_sz_mem + sizeof(ch_info *));
+    if (!pTmp) {
+      return NULL;
     }
+    a->extra_chns = pTmp;
+    a->extra_chns[old_sz] = (ch_info *)calloc(1, sizeof(ch_info));
+    if (!a->extra_chns[old_sz]) {
+      // if the realloc succeeds, but the calloc fails, we don't attempt to shrink the realloc back down
+      // by not recording the change to the size, we allow the next call to this function to realloc the
+      // block to presumably the same size it already is, which should be fine and not result in any leaks.
+      return NULL;
+    }
+
+    a->extra_sz += 1;
+    return a->extra_chns[old_sz];
   }
 
   return NULL;
 }
 
 void ch_map_clean(ch_map *map) {
-  if (map) {
-    size_t index;
-    for (index = 0; index < CH_MAP_HASH_SIZE; ++index) {
+  if (!map) {
+    return;
+  }
 
-      ch_map_array *a = &(map->table[index]);
+  for (size_t index = 0; index < CH_MAP_HASH_SIZE; ++index) {
+    ch_map_array *a = &(map->table[index]);
 
-      size_t i;
-      for (i = 0; i < CH_MAP_ARRAY_SIZE; ++i) {
-        ch_info *chi = &(a->main_chns[i]);
-        if (chi->allocated) {
-          ch_info_clean(chi);
-        }
+    for (size_t i = 0; i < CH_MAP_ARRAY_SIZE; ++i) {
+      ch_info *chi = &(a->main_chns[i]);
+      if (chi->allocated) {
+        ch_info_clean(chi);
       }
+    }
 
-      if (a->extra_chns) {
-        size_t sz = a->extra_sz;
-        for (i = 0; i < sz; ++i) {
-          ch_info *chi = a->extra_chns[i];
-          if (chi) {
-            if (chi->allocated) {
-              ch_info_clean(chi);
-            }
-            free(chi);
-            a->extra_chns[i] = NULL;
+    if (a->extra_chns) {
+      for (size_t i = 0; i < a->extra_sz; ++i) {
+        ch_info *chi = a->extra_chns[i];
+        if (chi) {
+          if (chi->allocated) {
+            ch_info_clean(chi);
           }
+          free(chi);
+          a->extra_chns[i] = NULL;
         }
-        free(a->extra_chns);
-        a->extra_chns = NULL;
       }
-      a->extra_sz = 0;
+      free(a->extra_chns);
+      a->extra_chns = NULL;
     }
+    a->extra_sz = 0;
   }
 }
 
 ////////////////// TCP connections ///////////////////////////////
 
 static void set_new_tc_id(uint8_t server_id, tcp_connection *tc) {
-  allocation *a = (allocation *)(tc->owner);
-  ur_map *map = a->tcp_connections;
-  uint32_t newid;
-  uint32_t sid = server_id;
-  sid = sid << 24;
+  const uint32_t sid = ((uint32_t)server_id) << 24;
+  allocation *const a = (allocation *)(tc->owner);
+  if (!a || !a->tcp_connections) {
+    return;
+  }
+
+  uint32_t newid = 0;
   do {
-    newid = 0;
-    while (!newid) {
-      newid = (uint32_t)turn_random();
-      if (!newid) {
-        continue;
-      }
-      newid = newid & 0x00FFFFFF;
-      if (!newid) {
-        continue;
-      }
-      newid = newid | sid;
-    }
-  } while (ur_map_get(map, (ur_map_key_type)newid, NULL));
+    do {
+      newid = ((uint32_t)turn_random()) & 0x00FFFFFF;
+    } while (!newid);
+    newid = newid | sid;
+  } while (ur_map_get(a->tcp_connections, (ur_map_key_type)newid, NULL));
   tc->id = newid;
-  ur_map_put(map, (ur_map_key_type)newid, (ur_map_value_type)tc);
+  ur_map_put(a->tcp_connections, (ur_map_key_type)newid, (ur_map_value_type)tc);
 }
 
 tcp_connection *create_tcp_connection(uint8_t server_id, allocation *a, stun_tid *tid, ioa_addr *peer_addr,
                                       int *err_code) {
   tcp_connection_list *tcl = &(a->tcs);
+  if (!tcl) {
+    return NULL;
+  }
+
   if (tcl->elems) {
-    size_t i;
-    for (i = 0; i < tcl->sz; ++i) {
+    for (size_t i = 0; i < tcl->sz; ++i) {
       tcp_connection *otc = tcl->elems[i];
       if (otc) {
         if (addr_eq(&(otc->peer_addr), peer_addr)) {
@@ -578,21 +572,24 @@ tcp_connection *create_tcp_connection(uint8_t server_id, allocation *a, stun_tid
       }
     }
   }
+
   tcp_connection *tc = (tcp_connection *)calloc(1, sizeof(tcp_connection));
+  if (!tc) {
+    return NULL;
+  }
   addr_cpy(&(tc->peer_addr), peer_addr);
   if (tid) {
     memcpy(&(tc->tid), tid, sizeof(stun_tid));
   }
   tc->owner = a;
 
-  int found = 0;
+  bool found = false;
   if (a->tcs.elems) {
-    size_t i;
-    for (i = 0; i < tcl->sz; ++i) {
+    for (size_t i = 0; i < tcl->sz; ++i) {
       tcp_connection *otc = tcl->elems[i];
       if (!otc) {
         tcl->elems[i] = tc;
-        found = 1;
+        found = true;
         break;
       }
     }
@@ -600,7 +597,11 @@ tcp_connection *create_tcp_connection(uint8_t server_id, allocation *a, stun_tid
 
   if (!found) {
     size_t old_sz_mem = a->tcs.sz * sizeof(tcp_connection *);
-    a->tcs.elems = (tcp_connection **)realloc(a->tcs.elems, old_sz_mem + sizeof(tcp_connection *));
+    tcp_connection **new_elems = realloc(a->tcs.elems, old_sz_mem + sizeof(tcp_connection *));
+    if (!new_elems) {
+      return NULL;
+    }
+    a->tcs.elems = new_elems;
     a->tcs.elems[a->tcs.sz] = tc;
     a->tcs.sz += 1;
     tcl = &(a->tcs);
@@ -611,38 +612,39 @@ tcp_connection *create_tcp_connection(uint8_t server_id, allocation *a, stun_tid
 }
 
 void delete_tcp_connection(tcp_connection *tc) {
-  if (tc) {
-    if (tc->done) {
-      TURN_LOG_FUNC(TURN_LOG_LEVEL_INFO, "!!! %s: check on already closed tcp data connection: %p\n", __FUNCTION__, tc);
-      return;
-    }
-    tc->done = 1;
+  if (!tc) {
+    return;
+  }
 
-    clear_unsent_buffer(&(tc->ub_to_client));
+  if (tc->done) {
+    TURN_LOG_FUNC(TURN_LOG_LEVEL_INFO, "!!! %s: check on already closed tcp data connection: %p\n", __FUNCTION__, tc);
+    return;
+  }
+  tc->done = true;
 
-    IOA_EVENT_DEL(tc->peer_conn_timeout);
-    IOA_EVENT_DEL(tc->conn_bind_timeout);
-    allocation *a = (allocation *)(tc->owner);
-    if (a) {
-      ur_map *map = a->tcp_connections;
-      if (map) {
-        ur_map_del(map, (ur_map_key_type)(tc->id), NULL);
-      }
-      tcp_connection_list *tcl = &(a->tcs);
-      if (tcl->elems) {
-        size_t i;
-        for (i = 0; i < tcl->sz; ++i) {
-          if (tcl->elems[i] == tc) {
-            tcl->elems[i] = NULL;
-            break;
-          }
+  clear_unsent_buffer(&(tc->ub_to_client));
+
+  IOA_EVENT_DEL(tc->peer_conn_timeout);
+  IOA_EVENT_DEL(tc->conn_bind_timeout);
+  allocation *a = (allocation *)(tc->owner);
+  if (a) {
+    ur_map *map = a->tcp_connections;
+    if (map) {
+      ur_map_del(map, (ur_map_key_type)(tc->id), NULL);
+    }
+    tcp_connection_list *tcl = &(a->tcs);
+    if (tcl->elems) {
+      for (size_t i = 0; i < tcl->sz; ++i) {
+        if (tcl->elems[i] == tc) {
+          tcl->elems[i] = NULL;
+          break;
         }
       }
     }
-    IOA_CLOSE_SOCKET(tc->client_s);
-    IOA_CLOSE_SOCKET(tc->peer_s);
-    free(tc);
   }
+  IOA_CLOSE_SOCKET(tc->client_s);
+  IOA_CLOSE_SOCKET(tc->peer_s);
+  free(tc);
 }
 
 tcp_connection *get_and_clean_tcp_connection_by_id(ur_map *map, tcp_connection_id id) {
@@ -670,9 +672,7 @@ tcp_connection *get_tcp_connection_by_peer(allocation *a, ioa_addr *peer_addr) {
   if (a && peer_addr) {
     tcp_connection_list *tcl = &(a->tcs);
     if (tcl->elems) {
-      size_t i;
-      size_t sz = tcl->sz;
-      for (i = 0; i < sz; ++i) {
+      for (size_t i = 0; i < tcl->sz; ++i) {
         tcp_connection *tc = tcl->elems[i];
         if (tc) {
           if (addr_eq(&(tc->peer_addr), peer_addr)) {
@@ -685,16 +685,16 @@ tcp_connection *get_tcp_connection_by_peer(allocation *a, ioa_addr *peer_addr) {
   return NULL;
 }
 
-int can_accept_tcp_connection_from_peer(allocation *a, ioa_addr *peer_addr, int server_relay) {
+bool can_accept_tcp_connection_from_peer(allocation *a, ioa_addr *peer_addr, int server_relay) {
   if (server_relay) {
-    return 1;
+    return true;
   }
 
   if (a && peer_addr) {
-    return (get_from_turn_permission_hashtable(&(a->addr_to_perm), peer_addr) != NULL);
+    return get_from_turn_permission_hashtable(&(a->addr_to_perm), peer_addr) != NULL;
   }
 
-  return 0;
+  return false;
 }
 
 //////////////// Unsent buffers //////////////////////
@@ -702,8 +702,7 @@ int can_accept_tcp_connection_from_peer(allocation *a, ioa_addr *peer_addr, int
 void clear_unsent_buffer(unsent_buffer *ub) {
   if (ub) {
     if (ub->bufs) {
-      size_t sz;
-      for (sz = 0; sz < ub->sz; sz++) {
+      for (size_t sz = 0; sz < ub->sz; sz++) {
         ioa_network_buffer_handle nbh = ub->bufs[sz];
         if (nbh) {
           ioa_network_buffer_delete(NULL, nbh);
@@ -730,8 +729,7 @@ void add_unsent_buffer(unsent_buffer *ub, ioa_network_buffer_handle nbh) {
 ioa_network_buffer_handle top_unsent_buffer(unsent_buffer *ub) {
   ioa_network_buffer_handle ret = NULL;
   if (ub && ub->bufs && ub->sz) {
-    size_t sz;
-    for (sz = 0; sz < ub->sz; ++sz) {
+    for (size_t sz = 0; sz < ub->sz; ++sz) {
       if (ub->bufs[sz]) {
         ret = ub->bufs[sz];
         break;
@@ -743,8 +741,7 @@ ioa_network_buffer_handle top_unsent_buffer(unsent_buffer *ub) {
 
 void pop_unsent_buffer(unsent_buffer *ub) {
   if (ub && ub->bufs && ub->sz) {
-    size_t sz;
-    for (sz = 0; sz < ub->sz; ++sz) {
+    for (size_t sz = 0; sz < ub->sz; ++sz) {
       if (ub->bufs[sz]) {
         ub->bufs[sz] = NULL;
         break;

+ 10 - 8
src/server/ns_turn_allocation.h

@@ -37,6 +37,8 @@
 #include "ns_turn_maps.h"
 #include "ns_turn_msg.h"
 
+#include <stdbool.h>
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -92,7 +94,7 @@ struct _tcp_connection {
   ioa_timer_handle conn_bind_timeout;
   stun_tid tid;
   void *owner; // a
-  int done;
+  bool done;
   unsent_buffer ub_to_client;
 };
 
@@ -108,7 +110,7 @@ typedef struct _tcp_connection_list {
 
 typedef struct _ch_info {
   uint16_t chnum;
-  int allocated;
+  bool allocated;
   uint16_t port;
   ioa_addr peer_addr;
   turn_time_t expiration_time;
@@ -138,13 +140,13 @@ void ch_map_clean(ch_map *map);
 ////////////////////////////
 
 typedef struct _turn_permission_info {
-  int allocated;
+  bool allocated;
   lm_map chns;
   ioa_addr addr;
   turn_time_t expiration_time;
   ioa_timer_handle lifetime_ev;
   void *owner; // a
-  int verbose;
+  bool verbose;
   unsigned long long session_id;
 } turn_permission_info;
 
@@ -171,7 +173,7 @@ typedef struct _turn_permission_hashtable {
 #define ALLOC_INDEX_ADDR(addr) ALLOC_INDEX(((addr)->ss).sa_family)
 
 typedef struct _allocation {
-  int is_valid;
+  bool is_valid;
   stun_tid tid;
   turn_permission_hashtable addr_to_perm;
   relay_endpoint_session relay_sessions[ALLOC_PROTOCOLS_NUMBER];
@@ -197,8 +199,8 @@ void clear_allocation(allocation *a, SOCKET_TYPE socket_type);
 void turn_permission_clean(turn_permission_info *tinfo);
 
 void set_allocation_lifetime_ev(allocation *a, turn_time_t exp_time, ioa_timer_handle ev, int family);
-int is_allocation_valid(const allocation *a);
-void set_allocation_valid(allocation *a, int value);
+bool is_allocation_valid(const allocation *a);
+void set_allocation_valid(allocation *a, bool value);
 turn_permission_info *allocation_get_permission(allocation *a, const ioa_addr *addr);
 turn_permission_hashtable *allocation_get_turn_permission_hashtable(allocation *a);
 turn_permission_info *allocation_add_permission(allocation *a, const ioa_addr *addr);
@@ -216,7 +218,7 @@ void set_allocation_family_invalid(allocation *a, int family);
 tcp_connection *get_and_clean_tcp_connection_by_id(ur_map *map, tcp_connection_id id);
 tcp_connection *get_tcp_connection_by_id(ur_map *map, tcp_connection_id id);
 tcp_connection *get_tcp_connection_by_peer(allocation *a, ioa_addr *peer_addr);
-int can_accept_tcp_connection_from_peer(allocation *a, ioa_addr *peer_addr, int server_relay);
+bool can_accept_tcp_connection_from_peer(allocation *a, ioa_addr *peer_addr, int server_relay);
 tcp_connection *create_tcp_connection(uint8_t server_id, allocation *a, stun_tid *tid, ioa_addr *peer_addr,
                                       int *err_code);
 void delete_tcp_connection(tcp_connection *tc);