ソースを参照

addressed possible null pointer dereferences (#1729)

this pr aims to address more static code analyser warnings, specifically
null pointer dereferences. the majority of changes are solely to quieten
the analyser, as `malloc` and `calloc` are unlikely to fail, but this
should at least lead to the code analysis being more readable and
usable.

where functions addressed had existing failure strategies, they were
used, however some functions will now silently fail rather than
attempting to dereference a null pointer. if there is a preferred
solution in these cases, i will be happy to implement it.

---

-
[27](https://github.com/redraincatching/coturn/security/code-scanning/27):
moved use of pointer inside `else` block of null check
-
[42](https://github.com/redraincatching/coturn/security/code-scanning/42):
added early return in case of null pointer
-
[69](https://github.com/redraincatching/coturn/security/code-scanning/69):
added null pointer check after `malloc`
-
[76](https://github.com/redraincatching/coturn/security/code-scanning/76):
added null pointer check after `calloc`
-
[77](https://github.com/redraincatching/coturn/security/code-scanning/77):
added null pointer check to loop guard
-
[82](https://github.com/redraincatching/coturn/security/code-scanning/82):
added null pointer check after `malloc`
-
[83](https://github.com/redraincatching/coturn/security/code-scanning/83):
added null pointer check after `malloc`
-
[84](https://github.com/redraincatching/coturn/security/code-scanning/84):
added null pointer check after `calloc`
-
[85](https://github.com/redraincatching/coturn/security/code-scanning/85):
added null pointer check around pointer use, as done earlier in the same
function
-
[86](https://github.com/redraincatching/coturn/security/code-scanning/86):
added null pointer check after `calloc`
-
[90](https://github.com/redraincatching/coturn/security/code-scanning/90)/[91](https://github.com/redraincatching/coturn/security/code-scanning/91)/[92](https://github.com/redraincatching/coturn/security/code-scanning/92)/[93](https://github.com/redraincatching/coturn/security/code-scanning/93):
added null pointer check to block
-
[94](https://github.com/redraincatching/coturn/security/code-scanning/94)/[95](https://github.com/redraincatching/coturn/security/code-scanning/95):
added null pointer checks after `malloc`
-
[108](https://github.com/redraincatching/coturn/security/code-scanning/108):
added check after `calloc`
-
[114](https://github.com/redraincatching/coturn/security/code-scanning/114):
added check after `memcpy`
-
[129](https://github.com/redraincatching/coturn/security/code-scanning/129):
added check after `calloc`
-
[145](https://github.com/redraincatching/coturn/security/code-scanning/145):
added check to if guard
-
[146](https://github.com/redraincatching/coturn/security/code-scanning/146):
added check to if guard
-
[154](https://github.com/redraincatching/coturn/security/code-scanning/154):
added early exit with error
-
[165](https://github.com/redraincatching/coturn/security/code-scanning/165):
added check after `malloc`
-
[170](https://github.com/redraincatching/coturn/security/code-scanning/170):
added early null return on null pointer
-
[171](https://github.com/redraincatching/coturn/security/code-scanning/171):
added check after `calloc`

---
![You're dereferencing a null
pointer!](https://i.makeagif.com/media/9-29-2015/YwGqu_.gif)
redraincatching 1 ヶ月 前
コミット
b1dddb5f49

+ 8 - 3
src/apps/common/apputils.c

@@ -919,6 +919,9 @@ static char *_WTA(__in wchar_t *pszInBuf, __in int nInSize, __out char **pszOutB
   // and we have to add space to the allocation ourselves.
   (*pnOutSize)++;
   *pszOutBuf = malloc(*pnOutSize * sizeof(char));
+  if (!pszOutBuf) {
+    return NULL;
+  }
   if (WideCharToMultiByte((UINT)0, (DWORD)0, pszInBuf, nInSize, *pszOutBuf, *pnOutSize, NULL, NULL) == 0) {
     free(*pszOutBuf);
     return NULL;
@@ -1340,10 +1343,12 @@ void build_base64_decoding_table(void) {
 
   char *table = (char *)calloc(256, sizeof(char));
 
-  for (size_t i = 0; i < 64; i++) {
-    table[(unsigned char)encoding_table[i]] = i;
+  if (table) {
+    for (size_t i = 0; i < 64; i++) {
+      table[(unsigned char)encoding_table[i]] = i;
+    }
+    decoding_table = table;
   }
-  decoding_table = table;
 }
 
 unsigned char *base64_decode(const char *data, size_t input_length, size_t *output_length) {

+ 8 - 7
src/apps/relay/ns_ioalib_engine_impl.c

@@ -594,11 +594,13 @@ ioa_timer_handle set_ioa_timer(ioa_engine_handle e, int secs, int ms, ioa_timer_
 
     tv.tv_sec = secs;
 
-    te->ctx = ctx;
-    te->e = e;
-    te->ev = ev;
-    te->cb = cb;
-    te->txt = strdup(txt);
+    if (te) {
+      te->ctx = ctx;
+      te->e = e;
+      te->ev = ev;
+      te->cb = cb;
+      te->txt = strdup(txt);
+    }
 
     if (!ms) {
       tv.tv_usec = 0;
@@ -1364,7 +1366,6 @@ ioa_socket_handle create_ioa_socket_from_fd(ioa_engine_handle e, ioa_socket_raw
   }
 
   ret->magic = SOCKET_MAGIC;
-
   ret->fd = fd;
   ret->st = st;
   ret->sat = sat;
@@ -3661,7 +3662,7 @@ void turn_report_allocation_set(void *a, turn_time_t lifetime, int refresh) {
           }
         }
 #if !defined(TURN_NO_HIREDIS)
-        if (e && ss) {
+        if (e && ss && ss->client_socket) {
           char key[1024];
           if (ss->realm_options.name[0]) {
             snprintf(key, sizeof(key), "turn/realm/%s/user/%s/allocation/%018llu/status", ss->realm_options.name,

+ 12 - 10
src/apps/relay/turn_admin_server.c

@@ -1178,20 +1178,22 @@ static void cliserver_input_handler(struct evconnlistener *l, evutil_socket_t fd
     return;
   }
 
-  clisession->rp = get_realm(NULL);
+  if (clisession) {
+    clisession->rp = get_realm(NULL);
 
-  set_socket_options_fd(fd, TCP_SOCKET, sa->sa_family);
+    set_socket_options_fd(fd, TCP_SOCKET, sa->sa_family);
 
-  clisession->fd = fd;
+    clisession->fd = fd;
 
-  addr_cpy(&(clisession->addr), (ioa_addr *)sa);
+    addr_cpy(&(clisession->addr), (ioa_addr *)sa);
 
-  clisession->bev = bufferevent_socket_new(adminserver.event_base, fd, TURN_BUFFEREVENTS_OPTIONS);
-  bufferevent_setcb(clisession->bev, cli_socket_input_handler_bev, NULL, cli_eventcb_bev, clisession);
-  bufferevent_setwatermark(clisession->bev, EV_READ | EV_WRITE, 0, BUFFEREVENT_HIGH_WATERMARK);
-  bufferevent_enable(clisession->bev, EV_READ); /* Start reading. */
+    clisession->bev = bufferevent_socket_new(adminserver.event_base, fd, TURN_BUFFEREVENTS_OPTIONS);
+    bufferevent_setcb(clisession->bev, cli_socket_input_handler_bev, NULL, cli_eventcb_bev, clisession);
+    bufferevent_setwatermark(clisession->bev, EV_READ | EV_WRITE, 0, BUFFEREVENT_HIGH_WATERMARK);
+    bufferevent_enable(clisession->bev, EV_READ); /* Start reading. */
 
-  clisession->ts = telnet_init(cli_telopts, cli_telnet_event_handler, 0, clisession);
+    clisession->ts = telnet_init(cli_telopts, cli_telnet_event_handler, 0, clisession);
+  }
 
   if (!(clisession->ts)) {
     const char *str = "Cannot open telnet session\n";
@@ -1449,7 +1451,7 @@ void admin_server_receive_message(struct bufferevent *bev, void *ptr) {
   int n = 0;
   struct evbuffer *input = bufferevent_get_input(bev);
 
-  while ((n = evbuffer_remove(input, tsi, sizeof(struct turn_session_info))) > 0) {
+  while (tsi && (n = evbuffer_remove(input, tsi, sizeof(struct turn_session_info))) > 0) {
     if (n != sizeof(struct turn_session_info)) {
       fprintf(stderr, "%s: Weird CLI buffer error: size=%d\n", __FUNCTION__, n);
       continue;

+ 1 - 1
src/apps/uclient/startuclient.c

@@ -289,7 +289,7 @@ start_socket:
     STRCPY(clnet_info->ifname, (const char *)ifname);
   }
 
-  if (use_secure) {
+  if (use_secure && clnet_info) {
     bool try_again = false;
     clnet_info->ssl = tls_connect(clnet_info->fd, &remote_addr, &try_again, connect_cycle++);
     if (!clnet_info->ssl) {

+ 1 - 1
src/apps/uclient/uclient.c

@@ -1489,7 +1489,7 @@ void start_mclient(const char *remote_address, int port, const unsigned char *if
 
     if (is_TCP_relay()) {
       if (passive_tcp) {
-        if (elems[i]->pinfo.is_peer) {
+        if (elems && elems[i]->pinfo.is_peer) {
           int connect_err = 0;
           socket_connect(elems[i]->pinfo.fd, &(elems[i]->pinfo.remote_addr), &connect_err);
         }

+ 4 - 0
src/client/ns_turn_msg.c

@@ -185,6 +185,10 @@ bool stun_produce_integrity_key_str(const uint8_t *uname, const uint8_t *realm,
     return false;
   }
 
+  if (!str) {
+    return false;
+  }
+
   strncpy((char *)str, (const char *)uname, sz);
   str[ulen] = ':';
   strncpy((char *)str + ulen + 1, (const char *)realm, sz - ulen - 1);

+ 6 - 4
src/server/ns_turn_maps.c

@@ -951,13 +951,15 @@ static bool ur_string_map_init(ur_string_map *map) {
 
 ur_string_map *ur_string_map_create(ur_string_map_func del_value_func) {
   ur_string_map *map = (ur_string_map *)malloc(sizeof(ur_string_map));
-  if (!ur_string_map_init(map)) {
+  if (map == NULL) {
+    return NULL;
+  } else if (!ur_string_map_init(map)) {
     free(map);
     return NULL;
+  } else {
+    map->del_value_func = del_value_func;
+    return map;
   }
-  assert(map);
-  map->del_value_func = del_value_func;
-  return map;
 }
 
 bool ur_string_map_put(ur_string_map *map, const ur_string_map_key_type key, ur_string_map_value_type value) {