1

I have a php page that contains a submit button, there are 2 ajax requests. The first ajax request submits to password_check.php, which if the password is good, you're done. If its bad, it gives an alert. :

function checkpassword( $password,  $error )
    { $.ajax({ 
                type:   'post', 
                url:    'password_check.php', 
                data:   $password.serialize(), 
                dataType: "text",
                success: 
                    function( data ){
                        if($error == 0 && data == "good"){
                            $password.addClass('good');
                            window.location.href = "finished.php";}
                        else if( data == "bad" ){
                            $password.addClass('bad');
                            alert("Your password is wrong.);}}});

The second ajax request, which always happens regardless of whether the password is good or bad, logs the ip address,name, date and time that the request is made (log_this.php). (These ajax requests are in the same function checkpassword).

        var $form= $( ".submit" );
        $.ajax({ 
                type:   'post', 
                url:    'log_this.php', 
                data:   $form.serialize(),
                dataType: "text", 
                success: function(data){},
                error:function( data ){}
            });
    }

Check password function call, where it starts.

checkpassword($(".submit"), $error);

password_check.php:

<?php 
// include database $connection file

$user = getuser();
$password = $_POST['submit_form'];
$query = "SELECT password 
         FROM   user_table WHERE  person = '$user'";
$returned_value = mysqli_query($connection, $query);

$data = $returned_value->fetch_assoc();

$realpassword = $data['password'];

if($password == $realpassword){ echo "good";}
else { echo "bad"; }
?>

log_this.php:

<?php 
// include database $connection file

$user=  getuser();

$first = $_POST['first'];
$last = $_POST[ 'last' ] ) );
$password = $_POST['password'];
$ip   = $_SERVER['REMOTE_ADDR'];
$long= ip2long($ip);


$query= "SELECT password, first, last FROM  user_table
         WHERE  person = '$user'";

$returned_value = mysqli_query($connection, $query);

$data = $returned_value->fetch_assoc();

$realpassword = $data['realpassword'];

$realfirst =  $data['first'];
$reallast  =   $data['nameLast'];
$is_true = ( $password == $realpassword ) ? true : false;
$correct_name =    ( $first == $realfirst && $last == $reallast ) ? true : false;

date_default_timezone_set( 'UK/London' );
$date  = date('Y-m-d H:i:s');

$query= "INSERT INTO log_this

        (user, first, last, datetime, ip, good_name, good_password, password_entered )

        VALUES ('$user','$first','$last','$date','$long','$correct_name','$is_true','$password')";

$returned_value= mysqli_query($connection, $query);
?>

Now my question is, would for any reason, writes not be made or inserted into the log_this table? is there a locking issue? log_this uses innodb but user_table uses myisam. could there be any reason why a write/insert is not made into the log_this table? also note, that log_this does not allow null values.

Thanks for your help.

Masu
  • 1,568
  • 4
  • 20
  • 41
  • No, locking issues are unlikely. You have not checked for errors after the query. `if (!$returned_value) echo mysqli_error($connection);` Because you have not called `mysqli_real_escape_string($connection, )` on those input variables from `$_POST`, a single quote in any of them would break the query. This is also [a SQL injection vulnerability](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) Start by debugging the `mysqli_query()` call and check `mysqli_error()`. – Michael Berkowski Feb 15 '15 at 22:36
  • I notice that you are apparently storing user passwords in clear text, and logging their attempts in clear text. That is alarming. I would recommend reading [How do you use bcrypt for password hashing in PHP](https://stackoverflow.com/questions/4795385/how-do-you-use-bcrypt-for-hashing-passwords-in-php). It is not a good idea to store your users' original passwords. – Michael Berkowski Feb 15 '15 at 22:37
  • the passwords are stored as sha hashes, and does a sha1( password), i edited the code to make it more simple – Masu Feb 15 '15 at 22:38
  • i am now doing error code checking using monolog, basically now after the fact the problem ocurred, i am logging with monolog the actual query everytime it is sent to the database for inserting – Masu Feb 15 '15 at 22:39
  • 1
    ^^Okay, that's a start! Still, if you can invest the time, `password_hash()` is recommended as `sha1()` is no longer considered secure. I'll leave my comment because others are likely to come along and say the same thing I did. – Michael Berkowski Feb 15 '15 at 22:40
  • what happened, after the fact was that, there were around 800-1000 users, and say they are a 1000, there were 20 inserts missing, but i received an email that they confirmed and submitted the form, (it executes a php mail function in one of the calls) – Masu Feb 15 '15 at 22:40
  • so 20 inserts didnt happen, but all emails came in – Masu Feb 15 '15 at 22:42
  • 1
    Do you have `error_reporting` turned up to `E_ALL` and watching your server's error log? Unless you report or take a fail action after `$returned_value = mysqli_query(...)` your code will go right on executing and send mail, etc. – Michael Berkowski Feb 15 '15 at 22:43
  • error reporting value is 22519? and display_errors is set to on – Masu Feb 15 '15 at 22:50
  • That's some value less than `E_ALL` (32767). Set it to `E_ALL` to test this. Most easily done at the top of the script with `error_reporting(E_ALL);` – Michael Berkowski Feb 15 '15 at 22:56
  • hmm i can't do that because the site is already in production – Masu Feb 15 '15 at 23:04
  • it might break pages on certain pages that were working before, and it would display all errors on the page – Masu Feb 15 '15 at 23:06
  • Turn off display_errors but log all errors. display_errors should be off in production anyway. – Michael Berkowski Feb 15 '15 at 23:16
  • ok, thanks. is there anyway i can troubleshoot this issue? now that i have monologging enabled, i should be able to see the error code mysql_errno returns and what values the query contains. is there a way i could recreate this, by creating the same amount of load as in production? – Masu Feb 15 '15 at 23:22
  • i was going to ask if i should be catching exceptions in my code, and if there are times where built-in php functions throw exceptions but i found this: http://stackoverflow.com/questions/10607539/is-there-any-native-php-function-which-throws-an-built-in-exception – Masu Feb 15 '15 at 23:47
  • 1
    I don't know if you'll be able to create the fail conditions because I don't know enough about your code. If it happens commonly enough, you should be able to log it as you're now doing. Using `set_error_handler()` to throw `ErrorException` could help, but also could be extremely disruptive. With `display_errors` off and `error_reporting` on `E_ALL` with `log_errors` on, the web server's error log should collect anything failing with PHP (separate from the failed queries you are logging) – Michael Berkowski Feb 16 '15 at 12:43

0 Answers0