siva-msft / curl

Other
0 stars 0 forks source link

Potential security issue in lib/ftp.c: Unchecked return from initialization function #44

Open monocle-ai opened 4 years ago

monocle-ai commented 4 years ago

What is a Conditionally Uninitialized Variable? The return value of a function that is potentially used to initialize a local variable is not checked. Therefore, reading the local variable may result in undefined behavior.

3 instances of this defect were found in the following locations:

Instance 1 File : lib/ftp.c Function: Curl_GetFTPResponse https://github.com/siva-msft/curl/blob/0eda1cffe4f39fe489cd0e859817213df27aecf5/lib/ftp.c#L411 Code extract:

    }
    else if(result & CURL_CSELECT_IN) {
      infof(data, "Ctrl conn has data while waiting for data conn\n");
      Curl_GetFTPResponse(&nread, conn, &ftpcode); <------ HERE

      if(ftpcode/100 > 3)

How can I fix it? Correct reference usage found in lib/ftp.c at line 3377. https://github.com/siva-msft/curl/blob/0eda1cffe4f39fe489cd0e859817213df27aecf5/lib/ftp.c#L3377 Code extract:


      pp->response = Curl_now(); /* timeout relative now */

      result = Curl_GetFTPResponse(&nread, conn, &ftpcode); <------ HERE
      if(result)
        return result;

Instance 2 File : lib/ftp.c Function: Curl_inet_ntop https://github.com/siva-msft/curl/blob/0eda1cffe4f39fe489cd0e859817213df27aecf5/lib/ftp.c#L1057 Code extract:

      break;
#endif
    default:
      Curl_inet_ntop(sa->sa_family, &sa4->sin_addr, hbuf, sizeof(hbuf)); <------ HERE
      break;
    }

How can I fix it? Correct reference usage found in lib/connect.c at line 650. https://github.com/siva-msft/curl/blob/0eda1cffe4f39fe489cd0e859817213df27aecf5/lib/connect.c#L650 Code extract:

#ifdef ENABLE_IPV6
    case AF_INET6:
      si6 = (struct sockaddr_in6 *)(void *) sa;
      if(Curl_inet_ntop(sa->sa_family, &si6->sin6_addr, <------ HERE
                        addr, MAX_IPADR_LEN)) {
        unsigned short us_port = ntohs(si6->sin6_port);

Instance 3 File : lib/ftp.c Function: Curl_printable_address https://github.com/siva-msft/curl/blob/0eda1cffe4f39fe489cd0e859817213df27aecf5/lib/ftp.c#L3450 Code extract:

                 int port)
{
  char buf[256];
  Curl_printable_address(ai, buf, sizeof(buf)); <------ HERE
  infof(conn->data, "Connecting to %s (%s) port %d\n", newhost, buf, port);
}

How can I fix it? Correct reference usage found in lib/socks.c at line 785. https://github.com/siva-msft/curl/blob/0eda1cffe4f39fe489cd0e859817213df27aecf5/lib/socks.c#L785 Code extract:

      return CURLE_COULDNT_RESOLVE_HOST;
    }

    if(Curl_printable_address(hp, dest, sizeof(dest))) { <------ HERE
      size_t destlen = strlen(dest);
      msnprintf(dest + destlen, sizeof(dest) - destlen, ":%d", remote_port);
siva-msft commented 4 years ago

lgtm.