Yubico / yubico-pam

Yubico Pluggable Authentication Module (PAM)
https://developers.yubico.com/yubico-pam
BSD 2-Clause "Simplified" License
689 stars 114 forks source link

Mysql close connection after return #230

Open baimard opened 3 years ago

baimard commented 3 years ago

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 commented 3 years ago

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