-1

I am trying to build some functions that help me with my project in PHP. After submitting the form the attempt column value in the database is not increasing, and there are no errors after submitting. The SQL connection is working without any problems. I think the problem is with the function itself.

<?php

include 'config/config.php';
include './framework/escaper/Escaper.php';

use Framework\Escaper\Escaper;

if ($_SERVER['REQUEST_METHOD'] === 'POST') {
    
    if(isset($name,$surname)){
        
        $instance = new Escaper();
        $stmt = $instance->CrackAttempt([
            'usersTable'=>'moemen',
            'mailColumn'=>'name',
            'passColumn'=>'surname',
            'emailField'=>$_POST['name'],
            'passField'=>$_POST['surname']

        ]); 
        
    }
}
?>
<form action="" method="POST">

<input type="text" name="name">
<input type="text" name="surname">

<button type="submit">submit</button>
</form>


<?php

namespace Framework\Escaper;

use PDO;
use PDOException;

// Calculate the root directory path dynamically
$rootDir = __DIR__ . '/../../'; // Go back two steps from the current directory

include $rootDir . 'config/config.php';

class Escaper
{

    // Function to execute SQL queries safely using prepared statements
    public function antiInjection($query, $params = [])
    {
        global $dbConfig;

        try {
            $pdo = new PDO("mysql:host={$dbConfig['host']};dbname={$dbConfig['dbname']}", $dbConfig['username'], $dbConfig['password']);
            $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

            $stmt = $pdo->prepare($query);
            foreach ($params as $paramName => $paramValue) {
                $stmt->bindValue($paramName, $paramValue);
            }

            $stmt->execute();

            return $stmt;
        } catch (PDOException $e) {
            echo "Error: " . $e->getMessage();
            return null;
        } finally {
            unset($pdo);
        }
    }

    // Notice before using this u must create LoginAttempt column + the default must be equal to 0 to

    public function CrackAttempt(array $params)
    {
        // Extract values from the $params array using array destructuring
        [
            'usersTable' => $usersTable,
            'mailColumn' => $mailColumn,
            'passColumn' => $passColumn,
            'emailField' => $emailField,
            'passField' => $passField,
        ] = $params;

        $query = "SELECT * FROM $usersTable WHERE $mailColumn = :email AND $passColumn = :password";

        // Prepare the parameters for the statement
        $preparedParams = [
            ':email' => $emailField,
            ':password' => $passField,
        ];

        $this->antiInjection($query, $preparedParams); // Capture the returned statement object

        $userLoginAttemptQuery = "UPDATE $usersTable SET LoginAttempt = LoginAttempt + 1 WHERE $mailColumn = :email";
        // Prepare the parameters for the statement
        $preparedParams2 = [
            ':email' => $emailField,
        ];

        $this->antiInjection($userLoginAttemptQuery, $preparedParams2); // Capture the returned statement object

    }
}

Dharman
  • 30,962
  • 25
  • 85
  • 135
  • When you debug it, are the variables being passed through as they should be? – droopsnoot Aug 08 '23 at 17:36
  • 1
    It looks as if you're storing passwords in plain text there, though I'm not certain whether it's a password or a surname. If it's a password, storing in plain text is a really bad idea. – droopsnoot Aug 08 '23 at 17:38
  • 1
    **Warning:** You are wide open to [SQL Injections](https://php.net/manual/en/security.database.sql-injection.php) and should use parameterized **prepared statements** instead of manually building your queries. They are provided by [PDO](https://php.net/manual/pdo.prepared-statements.php) or by [MySQLi](https://php.net/manual/mysqli.quickstart.prepared-statements.php). Never trust any kind of input! Even when your queries are executed only by trusted users, [you are still in risk of corrupting your data](http://bobby-tables.com/). [Escaping is not enough!](https://stackoverflow.com/q/32391315) – Dharman Aug 08 '23 at 17:59
  • bro but im using prepared statements in my antiInjection function, the antiInjection function works here correctly , i debugged it , but when i want to update it 's not updating ```$this->antiInjection($query, $preparedParams);``` – moemen saadeh Aug 08 '23 at 18:12
  • 1
    Prepared statements are not a magic pill against SQL injection. While technically you are safe if you never put user input in SQL, your helper function makes it still possible to accidently introduce SQL injection. You should never concatenate variables into SQL. – Dharman Aug 08 '23 at 18:18
  • 1
    Your `antiInjection` is about 15 lines too long. You have a lot of unnecessary or even straight bad code. I am not saying this to make you feel bad, but rather to inform you that you have chosen some bad material to learn from. Wherever you learned this from is a bad place to learn from. – Dharman Aug 08 '23 at 18:56
  • Instead of a loop with `bindValue` you should just pass data into `execute`. – Dharman Aug 08 '23 at 18:57
  • You should never catch exceptions just to spit out the message and die. This is already what a normal error handler does and it does so without displaying sensitive data. – Dharman Aug 08 '23 at 18:58
  • `unset($pdo);` is completely redundant as this is the last statement in that method and the object never leaves the scope. – Dharman Aug 08 '23 at 18:58
  • I don't see what's the point of `SELECT` statement. You are not actually selecting anything. – Dharman Aug 08 '23 at 19:00
  • ok look when email and password is not correct i will add 1 to the LoginAttempt column for that user (update the default 0 ) now i will make some actions like if the LoginAttempt column becomes 5 i will do something .. – moemen saadeh Aug 08 '23 at 19:08
  • Ok, why not start with something easier and see if it works. Just three lines of code: create PDO connection, prepare, execute. Then add more and see where you made a mistake. – Dharman Aug 08 '23 at 19:10
  • but i did it in antiInjection function ,i did all things daynamically , when i insert or update or did any sql statement i use it for make all things safe , then i make CrackAttempt function for brute force attack – moemen saadeh Aug 08 '23 at 19:34

0 Answers0