-1

I have a script that posts form responses to a MySQL Database. However, it is on occasion posting twice to the table with seconds between each row.

Here is the handling script:

if (isset($_POST['SignIn']))
    {
        $Name = $_POST['Name'];
        $Sleep = $_POST['Sleep'];
        $Soreness = $_POST['Soreness'];
        $Fatigue = $_POST['Fatigue'];
        $Energy = $_POST['Energy'];
        $Stress = $_POST['Stress'];
        $Total = $Sleep + $Soreness + $Fatigue + $Energy + $Stress;
        $Comments = $_POST['Comments'];

        $sql = "
            INSERT INTO
                YDP_Wellbeing (Name, Sleep, Soreness, Fatigue, Energy, Stress, Total, Comments)
            VALUES
                ('$Name', $Sleep, $Soreness, $Fatigue, $Energy, $Stress, $Total, '$Comments')";

        if (mysqli_query($con, $sql))
        {
            $_SESSION['alert-type'] = 'success';
            $_SESSION['alert-head'] = 'Welcome!';
            $_SESSION['alert-body'] = 'Thank You <strong>' . $Name . '</strong> You\'re Response Has Been Submitted.';
            header("location: index.php");
        }
        else
        {
            echo "Failed: " . mysqli_error($con);
        }
    }

Sometimes it posts once, other times it posts twice, so the issue is intermittent it would appear?

Barmar
  • 741,623
  • 53
  • 500
  • 612
Ashley
  • 21
  • 2
  • 2
    Sounds like the code is sometimes being invoked twice. Could be as simple as a user clicking a button again while waiting for a moment, or perhaps reloading a page that processed a form post. – David Oct 10 '19 at 20:15
  • 1
    Side note: Your code is *wide open* to **SQL injection**. Which is both a security problem and a common source of bugs. You'll want to learn about SQL injection here: https://www.php.net/manual/en/security.database.sql-injection.php As well as information and examples on how to prevent it here: https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php – David Oct 10 '19 at 20:17
  • @David is correct. There's nothing here that will insert twice, so the user must be calling it twice. – Barmar Oct 10 '19 at 20:35
  • I always add a `die();` after a redirect to prevent the script from continuing. See https://stackoverflow.com/questions/8665985/php-utilizing-exit-or-die-after-headerlocation – Tim Morton Oct 10 '19 at 20:42
  • Have you verified that the redirect is actually working at all, or does it simply go back to the same page and it *looks* like it redirected? – Tim Morton Oct 11 '19 at 16:10

2 Answers2

0

The most likely cause of the inconsistent behavior is the failure to stop the script after the redirect. Without adding die() or exit(), the script will continue to execute.

Since you've enclosed the section in an if clause that tests for a POST submission, I assume that if it fails the test it will present a form to be submitted. Since the script fails to terminate after the redirect, it will print the form again (and may or may not redirect).

Therefore what is likely happening is your user is submitting the form; and then upon seeing the same form presented after submission either

  1. goes on his merry way,
  2. resubmits the form, or
  3. attempts to press the back button and resubmits the form accidentally
Tim Morton
  • 2,614
  • 1
  • 15
  • 23
-2

Your code is vulnerable to the followings Attacks

1.) sql injection Attack as you passed variable directly into sql queries. I have mitigated it using prepared statements

2.) Html Injections and XSS attack: you will need to ensure sanitization for form inputs. i used intval() for integers and strip_tags() for strings assuming that you want to strip out dangerous html.

3.) session fixation attack. I also eliminate that using session _regenerate_id()

Here is your code Re written

<?php
$servername = "localhost";
$username = "db username goes here";
$pass = "your db password here";
$db_name = "your db name here";

// Create connection
$conn = new mysqli($servername, $username, $pass, $db_name);

// Check connection
if ($conn->connect_error) {
    echo "Connection to db failed";
}

if(isset($_POST['SignIn'])) {

        $Name = strip_tags($_POST['Name']);
        $Sleep = intval($_POST['Sleep']);
        $Soreness = intval($_POST['Soreness']);
        $Fatigue = intval($_POST['Fatigue']);
        $Energy = intval($_POST['Energy']);
        $Stress = intval($_POST['Stress']);
        $Total = $Sleep + $Soreness + $Fatigue + $Energy + $Stress;
        $Comments = strip_tags($_POST['Comments']);


// prepare your data
// i stands for integers and s stands for string
$stmt = $conn->prepare("INSERT INTO YDP_Wellbeing (Name, Sleep, Soreness, Fatigue, Energy, Stress, Total, Comments) VALUES (?, ?, ?, ?, ?, ?, ?, ?)");
$stmt->bind_param("siiiiiis", $Name,$Sleep,$Soreness,$Fatigue,$Energy,$Stress,$Total,$Comments);
$stmt->execute();

if($stmt){
echo "data inserted successfully";

// create session
session_start();
// prevents session fixation attacks using session regenerate id
session_regenerate_id();

//
$_SESSION['alert-type'] = 'success';
            $_SESSION['alert-head'] = 'Welcome!';

            $_SESSION['alert-body'] = 'Thank You <strong>' . $Name . '</strong> You\'re Response Has Been Submitted.';

            header("location: index.php");
}else{
echo "data insertion failed";
}

}

$stmt->close();
$conn->close();
?>
Nancy Moore
  • 2,322
  • 2
  • 21
  • 38
  • Why `strip_tags()` and `intval()`? – Funk Forty Niner Oct 10 '19 at 21:02
  • Read no 2 sentence please. I just added it and I said assuming he wants to strip all html elements. Intval() I need to ensure that those values he wants to sum are integers. Thanks – Nancy Moore Oct 10 '19 at 21:08
  • That's for viewing purposes, not for preventing an sql injection when doing a query. Prepared statements takes care of that. – Funk Forty Niner Oct 10 '19 at 21:10
  • please read no 1 sentence above i stated in my answer. I said I used prepared statement to prevent sql injections. so I rewrite his code to use mysqli prepared statements – Nancy Moore Oct 10 '19 at 21:13
  • 1
    Why the rewrite? Whatever happened on their end, your answer IMHO, won't fix this. – Funk Forty Niner Oct 10 '19 at 21:15
  • Everything I wrote is just a replicate of his just that I used prepared statement to avoid sql injections. So are saying that I should just give an answer with that sql vulnerability code of his. – Nancy Moore Oct 10 '19 at 21:23
  • I believe I gave him an explanation in sentence 1 to 3 – Nancy Moore Oct 10 '19 at 21:25
  • 1
    While this is very useful information for the OP, it doesn't actually answer the question. – David Oct 11 '19 at 01:18
  • Hi, Thank you for this, I will take this into account. Do we know why this might be posting twice? This code is in a separate "handling" file to that of the form if that information helps? – Ashley Oct 11 '19 at 14:58