0

For a 'Confirm Order' page in application written in PHP I need to prevent multiple form submissions so we don't get duplicate orders. I have attempted to handle this in two ways:

  1. For users supporting javascript the submit button is disabled on click
  2. A token is generated with the form and stored in session. On the first submit they're compared and the session token is removed. Subsequent submissions without a matching token should therefore be rejected. This should have the added benefit of preventing CSRF attacks.

I understand that both are fairly standard practice, yet the issue seems to remain? With js disabled if I click the submit button several times I will get X number of duplicate orders.

This makes me think that perhaps the issue is config related. It is hosted on lighthttpd and php is compiled with cgi-fcgi. I;m not entirely sure if that is even relevant but I am baffled as to how this is now possible.

The server code is as follows (cut down for brevity):

<?php
    $_SESSION['token'] = uniqid('', true);
?>
<form name="myform" action="confirm" method="POST">
  <!--.... -->
 <input type="hidden" name="csrftoken" value="<?php echo $_SESSION['token']; ?>" />
 <input type="submit" name="submit" />

Then on submit the token is verified:

<?php
   if ($_POST['csrftoken'] == $_SESSION['token']) {
       //proceed and process order
       unset($_SESSION['token']);
   }

?>

Session is started and the token is correctly generated and subsequently unset.

I have used this approach in the past with no problems but this time round it still seems to get through. Would appreciate any insight.

John Royal
  • 371
  • 4
  • 13
  • 1
    Keep session open as short as possible and force saving with `session_write_close()`. It's a bad practice using `$_SESSION` for outputting html. Copy needed session data on a local var then release session. – Ghigo Mar 20 '13 at 08:52
  • Thanks. The app is a legacy application that I am fixing a bug on. I would not normally be referencing $_SESSION in HTML output. The lack session_write_close was indeed part of the problem. The issue being that the original form submission occurs form an iframe which locks the session so it doesn't get unset. Using session_write_close does get round this issue but introduces the further problem of subsequent session changes not being updated properly even if session_start is subsequently used. – John Royal Mar 21 '13 at 08:40
  • Code should be restructured so that all $_SESSION access happens between `session_start()` and `session_write_close()`. I understand that legacy code has to work, so you can consider a quite ugly hack: search all $_SESSION access after `session_write_close()`, reopen session, modify session data and close it again. Bad performance of course but it should do the trick. – Ghigo Mar 21 '13 at 09:46

1 Answers1

0

Following the comments from Ghigo I have refactored the code to make this work.

The issue was ultimately an issue with session locking from an iframe and the form submission. Making sure the session was closed as early as possible was key to fixing this.

John Royal
  • 371
  • 4
  • 13