mkulke / ftplibpp

Platform independent c++ library providing ftp client functionality.
GNU Lesser General Public License v2.1
291 stars 108 forks source link

Critical memory leak in int ftplib::FtpOpenPort #13

Open Fojtik opened 7 years ago

Fojtik commented 7 years ago

Original code:

  ftphandle *ctrl = static_cast<ftphandle*>(calloc(1,sizeof(ftphandle)));
  if (ctrl == NULL)
  {
    perror("calloc");
    net_close(sData);
    return -1;
  }
  if ((mode == 'A') && ((ctrl->buf = static_cast<char*>(malloc(FTPLIB_BUFSIZ))) == NULL))
  {
    perror("calloc");
    net_close(sData);
    free(ctrl);
    return -1;
  }

  if (!FtpSendCmd(cmd, '1', nControl))
  {
    FtpClose(*nData);
    *nData = NULL;
    return -1;     /// THIS CAUSES MEMORY LEAK IN CRTL!!!
  }

Proposed fix:

ftphandle *ctrl = static_cast<ftphandle*>(calloc(1,sizeof(ftphandle)));
  if (ctrl == NULL)
  {
    perror("calloc");
    net_close(sData);
    return -1;
  }
  if ((mode == 'A') && ((ctrl->buf = static_cast<char*>(malloc(FTPLIB_BUFSIZ))) == NULL))
  {
    perror("calloc");
    net_close(sData);
    free(ctrl);
    return -1;
  }

  if (!FtpSendCmd(cmd, '1', nControl))
  {
    FtpClose(*nData);
    *nData = NULL;
    free(ctrl);         // Fixed by JFO - potential memory leak.
    return -1;
  }
dcrivelli commented 1 year ago

FtpClose(*nData); cannot work as nData has not been assigned yet... you should better do like:

if (!FtpSendCmd(cmd, '1', nControl)) { FtpClose(ctrl); *nData = NULL; return -1; }