yubico-pam icon indicating copy to clipboard operation
yubico-pam copied to clipboard

Mysql close connection after return

Open baimard opened this issue 3 years ago • 1 comments

in util.c

  while(!mysql_stmt_fetch(stmt))
  {
    if(bind[0].is_null_value)
    {
      D (debug_file, "mysql_stmt_fetch() failed");
    }
    else
    {
      if(otp_id != NULL){
        if(int_data)
        {
          return AUTH_FOUND;
        }
        else
        {
          return AUTH_NOT_FOUND;
        }
      }
      else if(otp_id == NULL)
      {
        if(int_data)
        {
          return AUTH_NOT_FOUND;
        }
        else
        {
          return AUTH_NO_TOKENS;
        }
      }
    }
  }

  if(mysql_stmt_close(stmt))
  {
    if(verbose)
    D (debug_file, "mysql_stmt_close() failed %s", mysql_stmt_error(stmt));
    return retval;
  }

  mysql_close(con);
  mysql_library_end();

Close connection is after the return.

In production mode with lot of users, that can cause too many connection quickly on the database.

I will propose a fix in a pull request.

hot fix would be :

  if(mysql_stmt_close(stmt))
  {
    if(verbose)
    D (debug_file, "mysql_stmt_close() failed %s", mysql_stmt_error(stmt));
    return retval;
  }

  mysql_close(con);
  mysql_library_end();

  while(!mysql_stmt_fetch(stmt))
  {
    if(bind[0].is_null_value)
    {
      D (debug_file, "mysql_stmt_fetch() failed");
    }
    else
    {
      if(otp_id != NULL){
        if(int_data)
        {
          return AUTH_FOUND;
        }
        else
        {
          return AUTH_NOT_FOUND;
        }
      }
      else if(otp_id == NULL)
      {
        if(int_data)
        {
          return AUTH_NOT_FOUND;
        }
        else
        {
          return AUTH_NO_TOKENS;
        }
      }
    }
  }

But in case of errors before this, that can cause a security issue (DDOS).

baimard avatar Jul 23 '21 13:07 baimard

Pull request : https://github.com/Yubico/yubico-pam/pull/231

baimard avatar Jul 28 '21 09:07 baimard