php forms stop working as intended after checking for existence of some $_POST variables?

Question

I was adding edit functionality to a comment system that i was creating and i ran into an issue. I know that my code is quite vulnerable, but thats the way i want to create it! For now atleast :D
I have the edit form on page 'comments.inc.php'

echo "<form class='edit-form' method='POST' action='editcomment.php'>  
      <input type='hidden' name='cid' value='".$row['cid']."'>  
      <input type='hidden' name='uid' value='".$row['uid']."'>  
      <input type='hidden' name='date' value='".date('Y-m-d H:i:s')."'>  
      <input type='hidden' name='message' value='".$row['message']."'>  
      <button name='editSubmit'>Edit</button>  
      </form>";

I have another page 'editcomment.php' which looks like this

$cid = $_POST['cid'];  
$uid = $_POST['uid'];  
$date = $_POST['date'];  
$message = $_POST['message'];  
echo "<form method='POST' action='".editComments($conn)."'>  
      <input type='hidden' name='cid' value='".$cid."'>  
      <input type='hidden' name='uid' value='".$uid."'>  
      <input type='hidden' name='date' value='".$date."'>   
      <textarea name='message'>".$message." ".$_POST['editSubmit']."</textarea>
      <br>  
      <button name='editCommentSubmit'>Edit</button>  
      </form>";

The editComments($conn) function is again on 'comments.inc.php' and looks like this

function editComments($conn) {  
if (isset($_POST['editCommentSubmit'])) {
    $cid = $_POST['cid'];
    $uid = $_POST['uid'];
    $date = $_POST['date'];
    $message = $_POST['message'];

    $sql = "UPDATE comments SET message='$message', date='$date' WHERE cid='$cid'";
    $result = mysqli_query($conn, $sql);
    header("Location: index.php");
  }
}

This code works as expected but I wanted to restrict unauthorized access of 'editcomment.php' page, so i modified my 'editcomment.php' page like this

if (isset($_POST['editSubmit'])) {   
$cid = $_POST['cid'];    
$uid = $_POST['uid'];  
$date = $_POST['date'];  
$message = $_POST['message'];  
echo "<form method='POST' action='".editComments($conn)."'>  
    <input type='hidden' name='cid' value='".$cid."'>  
    <input type='hidden' name='uid' value='".$uid."'>  
    <input type='hidden' name='date' value='".$date."'>     
    <textarea name='message'>".$message."</textarea><br>  
    <button name='editCommentSubmit'>Edit</button>  
    </form>";  
}  
else {  
    header("Location: index.php?access=denied");  
    exit();  
}

But for some reason it doesn't work anymore, it keeps redirecting me to index.php?access=denied. Please Help me :(

So it redirects you to `index.php?access=denied` after you click submit on the form on the `editcomment.php` page? — Kvothe, Aug 13 '17 at 23:22
Yeah, after i click on editcomment.php page. Also it should be noted that it atleast doesnt authorize access if someone manually posts the link in browser. — crash_overdrive, Aug 13 '17 at 23:27
The problem is that your `$_POST` variable `editSubmit` is not set, as button **names** are not submitted. Also, you say you **want** to create vulnerable code? That's a first haha. Are you looking to exploit it yourself later, so that you can be called in at extra cost to fix it? ;) — Obsidian Age, Aug 13 '17 at 23:29
@ObsidianAge Oh hey, that makes sense! So how do i pass editCommentSubmit instead of editSubmit? And thats a pretty neat idea actually. I'll keep that in mind! — crash_overdrive, Aug 13 '17 at 23:30
@ObsidianAge but editSubmit is set! I can view the editcomment.php page at first which is possible only if $_POST['editSubmit'] was set. After making edits in it and clicking the edit button, it goes to index.php?access=denied and doesnt make any changes which were made in editcomment.php. — crash_overdrive, Aug 13 '17 at 23:42

score 0 · Accepted Answer · answered Aug 13 '17 at 23:45

The problem is that while forms do indeed require name attributes on the fields that you're sending through, name attributes for buttons are not passed through in $_POST. In fact, if you debug your $_POST data, you'll find that $_POST['editSubmit'] is not actually set.

Instead, what you're looking for is a hidden and pre-populated <input> on your comment-editing form comments.inc.php:

<input type="hidden" name="secret" value="anything" />

Then on your editcomment.php page, check for the presence of this <input>:

<?php

if ($_POST['secret']) {
  // You're allowed to edit a comment
}
else {
  // You're not coming from `comments.inc.php`
}

?>

Note that you also don't need to echo out your entire form inside a PHP echo. You can simply close the PHP, write the HTML, then start the PHP back up. This can even be done from inside a PHP conditional!

<?php
if (isset($_POST['secret'])) {
?>

<form method='POST' action='<?php editComments($conn); ?>'>  
<input type='hidden' name='cid' value='<?php echo $cid; ?>'>  
<input type='hidden' name='uid' value='<?php echo $uid; ?>'>  
<input type='hidden' name='date' value='<?php echo $date; ?>'>     
<textarea name='message'><?php echo $message; ?></textarea><br>  
<button name='editCommentSubmit'>Edit</button>  
</form>

<?php
} // Don't forget to close the conditional!
else {
  header("Location: index.php?access=denied");  
  exit();
}
?>

Depending on the amount of HTML you're outputting, this can make things significantly easier and tidier =]

Additionally, although you're using MySQLi, you're vulnerable to SQL injection. To avoid this, you should use prepared statements. Also ensure that your database user only has the required privileges. You can refer to this post for further information :)

Hope this helps! :)

also one more follow up question. I have used the name of buttons in isset() function before and they have worked. If i gave the value to a button (say 'first') and echoed out $_POST['ButtonName'] it has actually printed out 'first'. Additionally i have done it without setting value to a button and checking if ButtonName was set or not, and it was set every time! I'll never use that again, instead i'll use hidden inputs next time, but was genuinely curious as to why it was working before and not now! :O — crash_overdrive, Aug 14 '17 at 00:01

php forms stop working as intended after checking for existence of some $_POST variables?

1 Answers1