1

Im just wondering which is better to use for security.

Can this be written in php? Id like to switch some of the functions of my site to javascript.

var withdrawing;
function withdraw() {
  withdrawing=false;
  $.msgBox({
    title:"Withdraw Funds",
    content:"<div id=\"_withdraw_content\"><br><small>Enter valid <?php echo $settings['currency_sign']; ?> address:</small><br><input id=\"w_valid_ltc\" type='text' class='l' style='width: 100%;'><br><br><small>Enter amount to be paid-out:</small><br><input id=\"w_amount\" type='text' class='l' style='width: 100px; text-align: center;'><br><br><small><small>Min. value: <b>0.001</b> <?php echo $settings['currency_sign']; ?><br>We charge a fee <b>0.0002</b> <?php echo $settings['currency_sign']; ?> for each withdrawal.</small></small></div>",
    type:"info",
    opacity:0.8,
    buttons: [{ value: "Withdraw" }, { value: "Cancel" }],
    success: function(button) {
      if (button=="Withdraw" && withdrawing==false) {
        w_amount=$("input#w_amount").val();
        w_valid=$("input#w_valid_ltc").val();
        if (w_amount!='' && w_valid!='') {
          $("#_withdraw_content").html('<div style=\"height: 50px;\"></div>&nbsp;&nbsp;&nbsp;<img src="content/images/ajax_loader.gif">');
          withdrawing=true;
          _requestWithdraw(w_amount,w_valid);
        }
        else {
          alert('One of required fields stayed empty!');
        }
      }
    }
  });      
  return false;
}
James
  • 79
  • 1
  • 9
  • What is the definition of the `prot` function? – Kermit Apr 07 '14 at 01:27
  • It's possible this is vulnerable - it depends on what `prot()` is doing. –  Apr 07 '14 at 01:29
  • 1
    @Kermit and @Mike W: I assume prot() will be a substitute for `mysql_real_escape_string()`. @user14872: please consider a modern API like mysqli or PDO. The original mysql extension is deprecated. – VMai Apr 07 '14 at 01:40
  • Im just learning as we speak. Im not sure what prot() for. Im updating the post to include the front end to see if it helps anyone figure out how they are pulling out double their input. i wish it were a smaller first project, as this is making it harder to grasp due to so much information and the learning curve. – James Apr 07 '14 at 01:48
  • I have learned enough so far to know they might be changing the front end variables, but I havent learned enough to know what would be changed. The guy who coded this is unavailable, otherwise Id ask him how the heck someone can pull out double what they put in. Luckily I had enough to cover the loss and my users wont be effected. Ive pulled the site for maintenance until I can get it sorted. Any help would be appreciated. – James Apr 07 '14 at 01:52

1 Answers1

1

If the prot function escapes your input this code has no SQL injection issue.

What this code does have though is a synchronization bug. If someone runs this script multiple times at the same time it's possible that a transaction can happen multiple times - assuming your webserver runs multiple PHP threads, which it likely does. This also means a user can end up with a negative balance.

You need to implement some kind of locking to assure one seller can not initiate more than one transaction at a time.

File locks (using flock) are a simple and straightforward way to get a lock mutex in PHP.

Halcyon
  • 57,230
  • 10
  • 89
  • 128
  • Thats what I originally thought. That they were somehow doing multiple withdrawels at the same exact time, however looking through the database, I didnt see any negative balances which made me think the issue was bigger. Theri are a lot of smart people out there, Im hoping someone can give me a hand implementing what I need so i can get the site back up and running while I continue to read up on mysql and php. – James Apr 07 '14 at 02:00
  • Is the balance field signed or unsigned? (`int` vs `uint`) – Halcyon Apr 07 '14 at 02:01
  • The balance field in the database is "DOUBLE". But Throughout the script I dont see anything like that, but most of time it is called, it is called like this: $("#balance_").html(data['balance']); – James Apr 07 '14 at 02:22
  • Check the database datatype to see whether it is signed or unsigned. If the column datatype is unsigned, then it can't handle negative values to begin with, which depending on your setup may result in errors or warnings. You should be sure that possible exceptions at the database level are caught by the PHP logic as they may allow one side of the transaction to go through while the other doesn't. If this is happening, users may be exploiting it. – msg45f Apr 07 '14 at 02:40
  • Looking at the attributes section for the balance field, it is blank. Im not sure if that means its unsigned or signed. but my question about that is, do I want it to show a negative balance? I am assuming by the time the databse show the balance, the damage of withdrawing double what is put in, is already done? – James Apr 07 '14 at 02:46
  • Whether the damage is done depends on your current logic. Depending on your setup attempting to enter a negative value into an unsigned datatype may result in an error or a warning/unintended value. See [here](https://dev.mysql.com/doc/refman/5.5/en/out-of-range-and-overflow.html) for details. – msg45f Apr 07 '14 at 03:12