Commit 1514977b authored by Daniel Stenberg's avatar Daniel Stenberg
Browse files

security: simplify choose_mech

Coverity CID 1299424 identified dead code because of checks that could
never equal true (if the mechanism's name was NULL).

Simplified the function by removing a level of pointers and removing the
loop and array that weren't used.
parent fda0e74c
Loading
Loading
Loading
Loading
+19 −33
Original line number Diff line number Diff line
@@ -109,13 +109,6 @@ static char level_to_char(int level) {
  return 'P';
}

static const struct Curl_sec_client_mech * const mechs[] = {
#ifdef HAVE_GSSAPI
  &Curl_krb5_client_mech,
#endif
  NULL
};

/* Send an FTP command defined by |message| and the optional arguments. The
   function returns the ftp_code. If an error occurs, -1 is returned. */
static int ftp_send_command(struct connectdata *conn, const char *message, ...)
@@ -484,36 +477,29 @@ static CURLcode choose_mech(struct connectdata *conn)
{
  int ret;
  struct SessionHandle *data = conn->data;
  const struct Curl_sec_client_mech * const *mech;
  void *tmp_allocation;
  const char *mech_name;

  for(mech = mechs; (*mech); ++mech) {
    mech_name = (*mech)->name;
    /* We have no mechanism with a NULL name but keep this check */
    DEBUGASSERT(mech_name != NULL);
    if(mech_name == NULL) {
      infof(data, "Skipping mechanism with empty name (%p)\n", (void *)mech);
      continue;
    }
    tmp_allocation = realloc(conn->app_data, (*mech)->size);
  const struct Curl_sec_client_mech *mech = &Curl_krb5_client_mech;

  do {
    tmp_allocation = realloc(conn->app_data, mech->size);
    if(tmp_allocation == NULL) {
      failf(data, "Failed realloc of size %u", (*mech)->size);
      failf(data, "Failed realloc of size %u", mech->size);
      mech = NULL;
      return CURLE_OUT_OF_MEMORY;
    }
    conn->app_data = tmp_allocation;

    if((*mech)->init) {
      ret = (*mech)->init(conn->app_data);
      if(ret != 0) {
        infof(data, "Failed initialization for %s. Skipping it.\n", mech_name);
    if(mech->init) {
      ret = mech->init(conn->app_data);
      if(ret) {
        infof(data, "Failed initialization for %s. Skipping it.\n",
              mech->name);
        continue;
      }
    }

    infof(data, "Trying mechanism %s...\n", mech_name);
    ret = ftp_send_command(conn, "AUTH %s", mech_name);
    infof(data, "Trying mechanism %s...\n", mech->name);
    ret = ftp_send_command(conn, "AUTH %s", mech->name);
    if(ret < 0)
      /* FIXME: This error is too generic but it is OK for now. */
      return CURLE_COULDNT_CONNECT;
@@ -522,11 +508,11 @@ static CURLcode choose_mech(struct connectdata *conn)
      switch(ret) {
      case 504:
        infof(data, "Mechanism %s is not supported by the server (server "
                    "returned ftp code: 504).\n", mech_name);
                    "returned ftp code: 504).\n", mech->name);
        break;
      case 534:
        infof(data, "Mechanism %s was rejected by the server (server returned "
                    "ftp code: 534).\n", mech_name);
                    "ftp code: 534).\n", mech->name);
        break;
      default:
        if(ret/100 == 5) {
@@ -539,7 +525,7 @@ static CURLcode choose_mech(struct connectdata *conn)
    }

    /* Authenticate */
    ret = (*mech)->auth(conn->app_data, conn);
    ret = mech->auth(conn->app_data, conn);

    if(ret == AUTH_CONTINUE)
      continue;
@@ -549,7 +535,7 @@ static CURLcode choose_mech(struct connectdata *conn)
    }
    DEBUGASSERT(ret == AUTH_OK);

    conn->mech = *mech;
    conn->mech = mech;
    conn->sec_complete = 1;
    conn->recv[FIRSTSOCKET] = sec_recv;
    conn->send[FIRSTSOCKET] = sec_send;
@@ -559,10 +545,10 @@ static CURLcode choose_mech(struct connectdata *conn)
    /* Set the requested protection level */
    /* BLOCKING */
    (void)sec_set_protection_level(conn);
    break;
  }

  return *mech != NULL ? CURLE_OK : CURLE_FAILED_INIT;
  } WHILE_FALSE;

  return CURLE_OK;
}

CURLcode