-1

I have the following login.php file with the following content:

<?php
require 'db.php';
require '../functions/log_attempts.php';

session_start();

// Jeżeli sesja istnieje, przekierowuje do index.php
if(isset($_SESSION['IS_LOGIN'])) {
        header('Location: index.php');
}

if(isset($_POST['submit'])) {
    
    // Limit prób logowania
    $bantime = time()-30;
    $ip_address = getIpAddr();
    
    // Sprawdzenie ilości prób logowania
    $check_attempts = mysqli_query($connect, "SELECT count(*) as total_count
                                        FROM login_attempts 
                                        WHERE log_times > $bantime 
                                        and ip='$ip_address'");
    $check_login_row = mysqli_fetch_assoc( $check_attempts);
    $total_count = $check_login_row['total_count'];

    if($total_count==3) {
        echo '<div class="alert-box error">Osiągnięto limit prób logowania. Spróbuj ponownie po 30 sekundach.</div>';
    } else {
        $login = mysqli_real_escape_string($connect, $_POST['login']);
        $password = mysqli_real_escape_string($connect, $_POST['password']);
    
        $sql = "SELECT * FROM accounts WHERE login = '".$login."'";
        $result = mysqli_query($connect, $sql);
        $numRows = mysqli_num_rows($result);

        if($numRows == 1) {
            $row = mysqli_fetch_assoc($result);
            if(password_verify($password, $row['password'])) {

                session_start();
                $_SESSION['IS_LOGIN'] = true;
                mysqli_query($connect, "DELETE FROM login_attempts WHERE ip='$ip_address'");
                $_SESSION['login'] = $row['login'];
                $_SESSION['password'] = $row['password'];
                header('Location: index.php');
                exit();
            } else {
                
                $total_count++;
                $rem_attempts = 3-$total_count;

                if ($rem_attempts==0) {
                   echo '<div class="alert-box error">
                    Osiągnięto limit prób logowania. Spróbuj ponownie po 30 sekundach.</div>';
                } else {
                    echo '<div class="alert-box error">
                    Nieprawidłowe dane logowania.<br>Pozostało prób: '.$rem_attempts.'</div>';
                }
                
                $try_time=time();
                mysqli_query($connect, "INSERT INTO login_attempts(id, ip, log_times) VALUES ('','".$ip_address."','".$try_time."')");
            }
        } else {
            echo '<div class="alert-box error">
            Nieprawidłowe dane logowania.</div>';
        }
    }
}
$connect->close();
?>

log_attempts.php:

<?php
require 'db.php';

function getIpAddr() {
    if (!empty($_SERVER['HTTP_CLIENT_IP'])){
        $ipAddr=$_SERVER['HTTP_CLIENT_IP'];
    } elseif (!empty($_SERVER['HTTP_X_FORWARDED_FOR'])){
        $ipAddr=$_SERVER['HTTP_X_FORWARDED_FOR'];
    } else {
        $ipAddr=$_SERVER['REMOTE_ADDR'];
    }
    return $ipAddr;
}


?>

and database:

CREATE TABLE `login_attempts` (
    `id` INT(11) NOT NULL,
    `ip` VARBINARY(16) NOT NULL,
    `log_times` BIGINT(20) NOT NULL
)
COLLATE='utf8_general_ci'
ENGINE=InnoDB
;

and after entering the wrong password to the existing login, it keeps writing that there are 2 attempts left, and no record to the login_attempts database is registered, what went wrong? I took the counter code from this source: http://phpgurukul.com/how-to-limit-login-attempt-using-php-and-mysql/

mlukr
  • 1
  • 2
  • 1
    You have `require 'db.php';` in both the function library and the main code. Pick a place and only one place – RiggsFolly May 30 '22 at 17:14
  • 1
    First use parametrized queries. Secondly, all is restored after 30s. And you ask to put an id of the database which is not a int. Make an autoincrement key. – JoelCrypto May 30 '22 at 17:15
  • 2
    Your script is open to [SQL Injection Attack](http://stackoverflow.com/questions/60174). Even [if you are escaping inputs, its not safe!](http://stackoverflow.com/questions/5741187) You should always use [prepared parameterized statements](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) in either the `MYSQLI_` or `PDO` API's instead of concatenating user provided values into the query. Never trust ANY user input! – RiggsFolly May 30 '22 at 17:15
  • 1
    Running `mysqli_real_escape_string($connect, $_POST['password']);` on a user entered password has the possibility of changing it. See the [accepted answer here](https://stackoverflow.com/a/36628423/2310830) and dont fiddle with the users password – RiggsFolly May 30 '22 at 17:18
  • @RiggsFolly, fixed. – mlukr May 30 '22 at 17:18
  • 1
    Little Bobby Tables is always needed: https://xkcd.com/327/ – GrafiCode May 30 '22 at 17:19
  • 1
    You also have more than one `session_start()` You should run that once per script and do it as the first thing done in the script. – RiggsFolly May 30 '22 at 17:21
  • Thanks for your suggestion, I improved the code in terms of SQL Injection but still the counter is not working properly, I added autoincrement to 'id', and removed one session_start(). – mlukr May 30 '22 at 17:24
  • If I and my 100 friends are all in the office behind a company router trying to login to your site, we will all be using the routers WAN ip address, thats 100 individuals all with the same IP, so this code cannot really be very useful surely – RiggsFolly May 30 '22 at 17:25
  • `log_times` could be a TIMESTAMP and then you have Date Time functionality you can apply to that column where a BIGINT may look the same, but wont do date and time stuff simply and automatically – RiggsFolly May 30 '22 at 17:27
  • 1
    ___General Note___ Any tutorial or code source that show you to use concatenated values in a database query should be treated with the respact it deserves.... **And completely ignored** Its normally click bait that just posts any old rubbish to get you to generate advert impressions – RiggsFolly May 30 '22 at 17:32

0 Answers0