0

I am making my website secure to SQL Injections, so I am using prepared statements. But do I also need this when updating value? I have a text input where value comes from the current row value. In button click, this is how I save the new value in the text input:

HTML:

<form method='post' action='xtest.php'>
<input type='hidden' name='hidden_id' value='{$row['id']}'> <!-- id without url ref: https://stackoverflow.com/a/32278030/14266656 -->
<td><input type='text' name='note' id='note' value='{$row['note']}'></td>
<td><input type='submit' name='btSubmit' value='Submit'></td>
</form>

PHP (xtest.php):

$action = $_POST['btSubmit'];

if ($action == "Submit") {
    $id = $_POST['hidden_id'];
    $note = $_POST['note'];
    
    $sql = "UPDATE table SET note = '$note' WHERE id = $id";
    
    if ($link->query($sql) === TRUE) {
        echo "Record updated successfully";
        
    } else {
        echo "Error updating record: " . $link->error;
        
    }
    $link->close();
}

Am I doing this secure, or do I need to change things up?

GMB
  • 216,147
  • 25
  • 84
  • 135
Erik Auranaune
  • 1,384
  • 1
  • 12
  • 27
  • Perhaps [this](https://stackoverflow.com/questions/1582161/how-does-a-preparedstatement-avoid-or-prevent-sql-injection) will help – Spectric Sep 19 '20 at 01:22
  • 1
    @Aniox Just wondering, is my current code good? Or should I use this one: https://pastebin.com/XtfX3mYG? – Erik Auranaune Sep 19 '20 at 01:32
  • I'm not a PHP developer, and therefore am not qualified to make any professional suggestions, but having some experience with PreparedStatements in other environments - it seems pretty good to me. – Spectric Sep 19 '20 at 01:33

1 Answers1

0

Am I doing this secure?

Are you using a parameterized query?

$sql = "UPDATE table SET note = '$note' WHERE id = $id";

No, you are not. You are concatenating variables in the query string.

Consider a malicious user posting the following parameters to your form:

note: 'foo'
id:   '0 or 1 = 1'

Your query string becomes:

UPDATE table SET note = 'foo' WHERE id = 0 OR 1 = 1

The WHERE condition is always true: all rows in your table are updated to 'foo', instead of one single row as you intended to.

Do use prepared statements and parameterized queries. Everytime you find yourself munging variables in the query string, that's a red flag. So:

$sql = "UPDATE table SET note = ? WHERE id = ?;

More about this can be read in this famous SO question.

GMB
  • 216,147
  • 25
  • 84
  • 135
  • Ok, so I tried doing it like this: https://pastebin.com/3rZSy6eE , but I get error message: `Fatal error: Uncaught Error: Call to undefined method mysqli_stmt::get_result() in ...:118 Stack trace: #0 {main} thrown in ... on line 118`. Line 118 is: `$result = $stmt->get_result();`. What am I doing wrong? – Erik Auranaune Sep 19 '20 at 02:56
  • @Andrei: are you sure that you are running mysqli? Maybe you are running PDO? – GMB Sep 19 '20 at 03:08
  • I am new to this, so how do I know? I am using phpmyadmin, and this is my database_connect.php: https://pastebin.com/sM5jGMdt – Erik Auranaune Sep 19 '20 at 10:21