-2

I am new to PHP, so please bear with me. I set up Xampp since I'm working on a project to create a CSRF vulnerable site. This will simulate a bank transfer that is vulnerable to it. I need to create a php page for the transfer. I already have the code mapped out, since it's a nearly ended project. I populated the database and made sure everything was fine in that part. But there seems to be some kind of a problem, as instead of adding to the value of the transfer, to the receiver , the database gets changed to the exact same value +1 for some reason. And I cannot figure this out.

$target = $_GET['TransferTarget'];
$amount = $_GET['TransferAmount'];
$target_query = "SELECT * FROM accounts acc WHERE (acc.AccountNumber = '$target')";
$target_result = mysqli_query($connection, $target_query);
$target_balance = mysqli_data_seek($target_result, 0);
$target_balance = $target_balance + $amount;
$source_balance = $source_balance - $amount;
$update_target_query = "UPDATE accounts SET accounts.Amount = '$target_balance' WHERE accounts.AccountNumber = '$target'";
$update_source_query = "UPDATE accounts SET accounts.Amount = '$source_balance' WHERE accounts.AccountNumber = '$source_account'";
$update_target_result = mysqli_query($connection, $update_target_query);
$update_source_result = mysqli_query($connection, $update_source_query);

Desired outcome would be for example if I transferred 100 dollars to an acc that already had 1000, to be 1100. The result I get for the same example, is 101 dollars in the end account. So, the targets account gets updated to 101 dollars. I can't seem to understand why this happens.

  • $source_balance is never set to anything. Shouldn't you fetch that before you try to subtract from it? – kainaw Jun 12 '19 at 12:49
  • 3
    Warning: Your code is *wide open* to **SQL injection**. This means you are executing any code your users send you. You can learn more about SQL injection here: https://www.php.net/manual/en/security.database.sql-injection.php And see examples of how to prevent it here: https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php – David Jun 12 '19 at 12:49
  • No, the record is already created in the database. In this case it's called amount , which corresponds to the amount of supposed money that someone has on the account. So, the update is supposed to add the money that someone else sends, not update it to the amount sent. Hope Im explaining it well enough. Thanks for your help – themasterishome Jun 12 '19 at 12:50
  • As for the question being asked... What do you observe when you debug this? Output the exact SQL queries you're executing. Do they contain the values you expect? If not, where did those values come from and why do you expect them to be different? – David Jun 12 '19 at 12:50
  • The source balance is set up and works and updates very well as it should. I just didn't put in the code, to not make it cluttered. But it is defined . mysqli_data_seek($result, 2); $finfo = mysqli_fetch_row($result); $source_account = $finfo[2]; $source_balance = $finfo[3]; – themasterishome Jun 12 '19 at 12:52
  • 1
    You can do this straight in MySql without doing multiple queries. `SET accounts.Amount = accounts.Amount + '$amount'` – aynber Jun 12 '19 at 12:52
  • David, I know the page is susceptible to many attacks, but that doesn't matter, since it is supposed to be a vulnerable page. – themasterishome Jun 12 '19 at 12:53
  • 1
    @themasterishome: *"but that doesn't matter"* - Unless that's the source of the bug(s) you're experiencing. SQL injection vulnerabilities aren't just a means of attack, they're also a common source of bugs. If you're not here to correct the bugs in your code then why are you here? It *might not* be the source of this bug, but you're going to need to provide the requested debugging information to confirm that. – David Jun 12 '19 at 12:54
  • David, you're right, but as I said, I'm new to stackoverflow, and I was suggested to not post all the code and to focus on where I thought was the problem. Sorry for the trouble. – themasterishome Jun 12 '19 at 12:57

2 Answers2

1

The mysqli_data_seek only returns true or false, based on whether the operation was a success.

The loose nature of php lets you add $amount to true, and figures that 100 + true = 101.

Instead of mysqli_data_seek, you probably want to use one of the fetch methods instead.

Stratadox
  • 1,291
  • 8
  • 21
-1

Inside your code

$target_balance = mysqli_data_seek($target_result, 0);

$target_balance will initialize as true or false because mysqli_data_seek() method return true or false that means 1 and 0 respectively.

That's why you are getting 101 if your transaction amount is 100.

Solution:

 $target_details = mysqli_data_seek($target_result, 0);
 $target_balance= $target_details['target_balance'];
 $source_balance= $target_details['source_balance'];

and Then use both variable as you are doing...

Warning: Your code is not protected it's velnerable using sql_injection. Read more about how to protect your code.

[Update]

 $target_result = mysqli_data_seek($target_result, 0);

  ///New line
  $target_result=mysqli_fetch_assoc($target_result);
  //End new line

 $target_balance= $target_details['target_balance'];
 $source_balance= $target_details['source_balance'];
Badrinath
  • 501
  • 5
  • 14
  • 1
    If `mysqli_data_seek` returns a boolean value then what is `$target_details['target_balance']` expected to do in this code? A boolean value is not an array. – David Jun 12 '19 at 13:12
  • 1
    David is right, this did not work. In fact it took that error to the source balance as well. – themasterishome Jun 12 '19 at 13:31