CSRF protection enhanced. To be tested...

TODO

@@ -1,7 +1,3 @@
-Issues :
-========
-* Check CSRF everywhere
-
 Improvements :
 ==============
 * User groups

inc/CSRF.inc.php

@@ -19,8 +19,13 @@
         if(session_id() == '')
             session_start();
 
-        if(isset($_SESSION[$name.'_token']) && isset($_SESSION[$name.'_token_time']) && isset($_POST['token'])) {
+        if(isset($_SESSION[$name.'_token']) && isset($_SESSION[$name.'_token_time']) && (isset($_POST['token']) || isset($_GET['token']))) {
-            if($_SESSION[$name.'_token'] == $_POST['token']) {
+            if(!empty($_POST['token']))
+                $token = $_POST['token'];
+            else
+                $token = $_GET['token'];
+
+            if($_SESSION[$name.'_token'] == $token) {
                 if($_SESSION[$name.'_token_time'] >= (time() - (int) $time))
                     return true;
             }

index.php

@@ -259,6 +259,7 @@
                 $user_id = $current_user->getId();
             }
 
+            if(checkToken(600, 'password')) {  
                 $user = new User();
                 $user = $user->load(array('id'=>$user_id), true);
                 $user->newJsonToken();
@@ -267,10 +268,16 @@
                 
                 header('location: index.php?do=password&'.$get_redir);
                 exit();
+            }
+            else {
+                $tpl->assign('error', $errors['token_error'][LANG]);
+                $tpl->draw('index');
+            }
             break;
 
         case 'delete_user':
             if($_GET['user_id'] != $current_user->getId()) {
+                if(checkToken(600, 'edit_users')) {
                     $user = new User();
                     $user->setId($_GET['user_id']);
                     $user->delete();
@@ -321,10 +328,16 @@
                     header('location: index.php?do=edit_users&'.$get_redir);
                     exit();
                 }
+                else {
+                    $tpl->assign('error', $errors['token_error'][LANG]);
+                    $tpl->draw('index');
+                }
+            }
             break;
 
         case 'edit_notice':
             if(isset($_POST['notice'])) {
+                if(checkToken(600, 'settings')) {
                     setNotice($_POST['notice']);
 
                     // Clear the cache
@@ -334,6 +347,10 @@
                     header('location: index.php?'.$get_redir);
                     exit();
                 }
+                else {
+                    $tpl->assign('error', $errors['token_error'][LANG]);
+                }
+            }
 
             $tpl->assign('show_settings', false);
             $tpl->draw('settings');
@@ -499,6 +516,7 @@
 
         case 'delete_invoice':
             if(!empty($_GET['id'])) {
+                if(checkToken(600, 'invoice')) {
                     $invoice = new Invoice();
                     $invoice = $invoice->load(array('id'=>(int) $_GET['id']), true);
 
@@ -523,7 +541,12 @@
                         exit();
                     }
                     else {
-                    $tpl->assign('error', $errors['unauthorized']);
+                        $tpl->assign('error', $errors['unauthorized'][LANG]);
+                        $tpl->draw('index');
+                    }
+                }
+                else {
+                    $tpl->assign('error', $errors['token_error'][LANG]);
                     $tpl->draw('index');
                 }
             }
@@ -536,6 +559,7 @@
         case 'confirm_payback':
             if(!empty($_GET['from']) && !empty($_GET['to']) && !empty($_GET['invoice_id']) && $_GET['from'] != $_GET['to']) {
                 if($_GET['to'] == $current_user->getId() || $current_user->getAdmin()) {
+                    if(checkToken(600, 'invoice')) {
                         $invoice = new Invoice();
                         $invoice = $invoice->load(array('id'=>(int) $_GET['invoice_id']), true);
 
@@ -569,10 +593,15 @@
 
                         header('location: index.php');
                         exit();
+                    }
+                    else {
+                        $tpl->assign('error', $errors['token_error'][LANG]);
+                        $tpl->draw('index');
+                    }
 
                 }
                 else {
-                    $tpl->assign('error', $errors['unauthorized']);
+                    $tpl->assign('error', $errors['unauthorized'][LANG]);
                     $tpl->draw('index');
                 }
             }
@@ -584,6 +613,7 @@
         case 'delete_payback':
             if(!empty($_GET['from']) && !empty($_GET['to']) && !empty($_GET['invoice_id'])) {
                 if($_GET['to'] == $current_user->getId() || $current_user->getAdmin()) {
+                    if(checkToken(600, 'invoice')) {
                         $paybacks = new Payback();
 
                         $paybacks = $paybacks->load(array('to_user'=>(int) $_GET['to'], 'from_user'=> (int) $_GET['from'], 'invoice_id'=> (int) $_GET['invoice_id']));
@@ -601,6 +631,12 @@
                         header('location: index.php');
                         exit();
                     }
+                    else {
+                        $tpl->assign('error', $errors['token_error'][LANG]);
+                        $tpl->draw('index');
+                    }
+
+                }
                 else {
                     header('location: index.php');
                     exit();
@@ -615,6 +651,7 @@
         case 'payall':
             if(!empty($_GET['from']) && !empty($_GET['to'])) {
                 if($_GET['to'] == $current_user->getId() || $current_user->getAdmin()) {
+                    if(checkToken(600, 'invoice')) {
                         // Confirm all paybacks when to is buyer
                         $invoices = new Invoice();
                         $invoices = $invoices->load(array('buyer'=>(int) $_GET['to']));
@@ -664,6 +701,12 @@
                         header('location: index.php');
                         exit();
                     }
+                    else {
+                        $tpl->assign('error', $errors['token_error'][LANG]);
+                        $tpl->draw('index');
+                    }
+
+                }
                 else {
                     header('location: index.php');
                     exit();
@@ -702,6 +745,7 @@
 
             $tpl->assign('list', true);
             $tpl->assign('global_paybacks', $global_paybacks);
+            $tpl->assign('payback', generateToken('global_payback'));
 
             $tpl->draw('see_paybacks');
             break;
@@ -709,6 +753,7 @@
         case "confirm_global_paybacks":
             if(!empty($_GET['from']) && !empty($_GET['to']) && !empty($_GET['payback_id']) && $_GET['from'] != $_GET['to']) {
                 if($_GET['to'] == $current_user->getId() || $current_user->getAdmin()) {
+                    if(checkToken(600, 'global_payback')) {
                         $global_payback = new GlobalPayback();
                         $global_payback = $global_payback->load(array('id'=>(int) $_GET['payback_id']), true);
 
@@ -734,10 +779,14 @@
 
                         header('location: ?do=see_paybacks&id='.(int)$_GET['payback_id']);
                         exit();
-
                     }
                     else {
-                    $tpl->assign('error', $errors['unauthorized']);
+                        header('location: index.php');
+                        exit();
+                    }
+                }
+                else {
+                    $tpl->assign('error', $errors['token_error'][LANG]);
                     $tpl->draw('index');
                 }
             }
@@ -765,6 +814,7 @@
             }
             else {
                 if(!empty($_POST['users_in'])) {
+                    if(checkToken(600, 'global_payback')) {
                         $global_payback = new GlobalPayback();
 
                         // Backup database
@@ -864,12 +914,18 @@
                         header('location: index.php?do=manage_paybacks&'.$get_redir);
                         exit();
                     }
+                    else {
+                        $tpl->assign('error', $errors['token_error'][LANG]);
+                        $tpl->draw('index');
+                    }
+                }
                 
                 $users_list = new User();
                 $users_list = $users_list->load();
 
                 $tpl->assign('users', $users_list);
             }
+            $tpl->assign('payback', generateToken('global_payback'));
             $tpl->draw('manage_paybacks');
             break;
 
@@ -971,6 +1027,9 @@
                 $tpl->assign('paybacks', secureDisplay($paybacks));
                 $tpl->assign('balances', secureDisplay($balances));
 
+                $tpl->assign('token', generate_token('invoice'));
+
+
                 // Cache the page (1 month to make it almost permanent and only regenerate it upon new invoice)
                 $tpl->cache('index', 108000, $current_user->getLogin().$_GET['all']);
 

tpl/default_en/edit_users.html

@@ -19,7 +19,7 @@
         <td>{$value->getDisplayName()}</td>
         <td>{$value->getAdmin() ? "Yes" : "No"}</td>
         <td><a href="index.php?do=edit_users&user_id={$value->getId()}">Edit</a></td>
-        <td>{if condition="$value->getId() != $current_user->getId()"}<a href="index.php?do=delete_user&user_id={$value->getId()}">Delete</a>{/if}</td>
+        <td>{if condition="$value->getId() != $current_user->getId()"}<a href="index.php?do=delete_user&amp;user_id={$value->getId()}&amp;token={$token}">Delete</a>{/if}</td>
     </tr>
     {/loop}
 </table>
@@ -52,7 +52,7 @@
 
 {if condition="$user_id != -1"}
     <h2>Personal token for this user</h2>
-    <p>The personal token for this user to be used with the API is : {$user_data->getJsonToken()}.<br/>If you think it might be compromised, you can <a href="index.php?do=new_token&user_id={$user_data->getId()}">generate a new one</a>.</p>
+    <p>The personal token for this user to be used with the API is : {$user_data->getJsonToken()}.<br/>If you think it might be compromised, you can <a href="index.php?do=new_token&user_id={$user_data->getId()}&token={$token}">generate a new one</a>.</p>
 {/if}
 
 {elseif condition="$view == 'password'"}

tpl/default_en/index.html

@@ -23,7 +23,7 @@
                     {elseif condition="$balances[$value1->getId()][$value2->getId()] === '-' || ($value2->getId() != $current_user->getId() && !$current_user->getAdmin())"}
                         <td>{$balances[$value1->getId()][$value2->getId()]}</td>
                     {else}
-                        <td><a href="?do=payall&amp;from={$value1->getId()}&amp;to={$value2->getId()}">{$balances[$value1->getId()][$value2->getId()]}</a></td>
+                        <td><a href="?do=payall&amp;from={$value1->getId()}&amp;to={$value2->getId()}&amp;token={$token}">{$balances[$value1->getId()][$value2->getId()]}</a></td>
                     {/if}
                 {/loop}
             </tr>
@@ -66,7 +66,7 @@
                                 - 
                                 {if condition="$paybacks[$value1->getId()] === false || !in_array($key2, array_keys($paybacks[$value1->getId()]))"}
                                     {if condition="$current_user->getId() == $value1->getBuyer() || $current_user->getAdmin()"}
-                                        <a href="?do=confirm_payback&amp;from={$key2}&amp;to={$value1->getBuyer()}&amp;invoice_id={$value1->getId()}" title="Confirm payback">
+                                        <a href="?do=confirm_payback&amp;from={$key2}&amp;to={$value1->getBuyer()}&amp;invoice_id={$value1->getId()}&amp;token={$token}" title="Confirm payback">
                                     {/if}
                                         Remains {$value1->getAmountPerPerson($key2)} {$currency}
                                     {if condition="$current_user->getId() == $value1->getBuyer() || $current_user->getAdmin()"}
@@ -75,7 +75,7 @@
                                 {else}
                                     {if condition="$paybacks[$value1->getId()][$key2]->getAmount() == $value1->getAmountPerPerson($key2)"}
                                         {if condition="$current_user->getId() == $value1->getBuyer() || $current_user->getAdmin()"}
-                                            <a href="?do=delete_payback&amp;from={$key2}&amp;to={$value1->getBuyer()}&amp;invoice_id={$value1->getId()}" title="Delete payback">
+                                            <a href="?do=delete_payback&amp;from={$key2}&amp;to={$value1->getBuyer()}&amp;invoice_id={$value1->getId()}&amp;token={$token}" title="Delete payback">
                                         {/if}
                                             Paid
                                         {if condition="$current_user->getId() == $value1->getBuyer() || $current_user->getAdmin()"}
@@ -83,7 +83,7 @@
                                         {/if}   
                                     {else}
                                         {if condition="$current_user->getId() == $value1->getBuyer() || $current_user->getAdmin()"}
-                                            <a href="?do=confirm_payback&amp;from={$key2}&amp;to={$value1->getBuyer()}&amp;invoice_id={$value1->getId()}&amp;payback_id={$paybacks[$value1->getId()][$key2]->getId()}" title="Confirm payback">
+                                            <a href="?do=confirm_payback&amp;from={$key2}&amp;to={$value1->getBuyer()}&amp;invoice_id={$value1->getId()}&amp;payback_id={$paybacks[$value1->getId()][$key2]->getId()}&amp;token={$token}" title="Confirm payback">
                                         {/if}
                                             Remains {$value1->getAmountPerPerson($key2) - $paybacks[$value1->getId()][$key2]->getAmount()}{$currency}
                                         {if condition="$current_user->getId() == $value1->getBuyer() || $current_user->getAdmin()"}
@@ -106,7 +106,7 @@
                     </td>
                     <td>
                         {if condition="$value->getBuyer() == $current_user->getId() || $current_user->getAdmin()"}
-                            <a href="index.php?do=delete_invoice&id={$value->getId()}">Delete</a>
+                            <a href="index.php?do=delete_invoice&id={$value->getId()}&token={$token}">Delete</a>
                         {else}
                             -
                         {/if}

tpl/default_en/manage_paybacks.html

@@ -37,6 +37,7 @@
     </fieldset>
     <p>
         <input type="submit" onclick="return confirm('You are going to add a global payback which includes the selected user. Confirm ?');" value="Confirm"/>
+        <input type="hidden" name="token" value="{$token}"/>
     </p>
     </form>
 {/if}

tpl/default_en/see_paybacks.html

@@ -8,7 +8,7 @@
                 <dt>{$value->getDate()}</dt>
                 <dd>
                     {if condition="$value->getClosed() !== false"}
-                         <a href="?do=see_paybacks&id={$value->getId()}" onclick="return confirm("Are you sure you want to confirm this payback ? This action can't be undone easily");">Payback n°{$value->getId()}</a>
+                         <a href="?do=see_paybacks&id={$value->getId()}">Payback n°{$value->getId()}</a>
                     {else}
                         [Closed] Payback n°{$value->getId()}
                     {/if}
@@ -41,7 +41,7 @@
                         {else}
                             <td>
                                 {if condition="$table[$key1][$key2] != 0"}
-                                    <a href="?do=confirm_global_paybacks&amp;from={$key1}&amp;to={$key2}&amp;payback_id={$global_paybacks->getId()}" title="Confirm payback">{$table[$key1][$key2]}</a>
+                                <a href="?do=confirm_global_paybacks&amp;from={$key1}&amp;to={$key2}&amp;payback_id={$global_paybacks->getId()}&amp;token={$token}" title="Confirm payback" onclick="return confirm("Are you sure you want to confirm this payback ? This action can't be undone easily");">{$table[$key1][$key2]}</a>
                                 {else}
                                     -
                                 {/if}

tpl/default_fr/edit_users.html

@@ -19,7 +19,7 @@
         <td>{$value->getDisplayName()}</td>
         <td>{$value->getAdmin() ? "Yes" : "No"}</td>
         <td><a href="index.php?do=edit_users&user_id={$value->getId()}">Modifier</a></td>
-        <td>{if condition="$value->getId() != $current_user->getId()"}<a href="index.php?do=delete_user&user_id={$value->getId()}">Supprimer</a>{/if}</td>
+        <td>{if condition="$value->getId() != $current_user->getId()"}<a href="index.php?do=delete_user&amp;user_id={$value->getId()}&amp;token={$token}">Supprimer</a>{/if}</td>
     </tr>
     {/loop}
 </table>
@@ -64,6 +64,6 @@
 </form>
 
 <h2>Votre jeton de connexion à l'API</h2>
-<p>Votre jeton personnel de connexion à l'API est : {$json_token}.<br/>Si vous pensez qu'il a pu être compromis, vous pouvez <a href="index.php?do=new_token">en générer un nouveau</a>.</p>
+<p>Votre jeton personnel de connexion à l'API est : {$json_token}.<br/>Si vous pensez qu'il a pu être compromis, vous pouvez <a href="index.php?do=new_token&amp;token={$token}">en générer un nouveau</a>.</p>
 {/if}
 {include="footer"}

tpl/default_fr/index.html

@@ -23,7 +23,7 @@
                     {elseif condition="$balances[$value1->getId()][$value2->getId()] === '-' || ($value2->getId() != $current_user->getId() && !$current_user->getAdmin())"}
                         <td>{$balances[$value1->getId()][$value2->getId()]}</td>
                     {else}
-                        <td><a href="?do=payall&amp;from={$value1->getId()}&amp;to={$value2->getId()}">{$balances[$value1->getId()][$value2->getId()]}</a></td>
+                        <td><a href="?do=payall&amp;from={$value1->getId()}&amp;to={$value2->getId()}&amp;token={$token}">{$balances[$value1->getId()][$value2->getId()]}</a></td>
                     {/if}
                 {/loop}
             </tr>
@@ -66,7 +66,7 @@
                                 - 
                                 {if condition="$paybacks[$value1->getId()] === false || !in_array($key2, array_keys($paybacks[$value1->getId()]))"}
                                     {if condition="$current_user->getId() == $value1->getBuyer() || $current_user->getAdmin()"}
-                                        <a href="?do=confirm_payback&amp;from={$key2}&amp;to={$value1->getBuyer()}&amp;invoice_id={$value1->getId()}" title="Confirmer le remboursement">
+                                        <a href="?do=confirm_payback&amp;from={$key2}&amp;to={$value1->getBuyer()}&amp;invoice_id={$value1->getId()}&amp;token={$token}" title="Confirmer le remboursement">
                                     {/if}
                                         Reste {$value1->getAmountPerPerson($key2)} {$currency}
                                     {if condition="$current_user->getId() == $value1->getBuyer() || $current_user->getAdmin()"}
@@ -75,7 +75,7 @@
                                 {else}
                                     {if condition="$paybacks[$value1->getId()][$key2]->getAmount() == $value1->getAmountPerPerson($key2)"}
                                         {if condition="$current_user->getId() == $value1->getBuyer() || $current_user->getAdmin()"}
-                                            <a href="?do=delete_payback&amp;from={$key2}&amp;to={$value1->getBuyer()}&amp;invoice_id={$value1->getId()}" title="Supprimer le remboursement">
+                                            <a href="?do=delete_payback&amp;from={$key2}&amp;to={$value1->getBuyer()}&amp;invoice_id={$value1->getId()}&amp;token={$token}" title="Supprimer le remboursement">
                                         {/if}
                                             Payé
                                         {if condition="$current_user->getId() == $value1->getBuyer() || $current_user->getAdmin()"}
@@ -83,7 +83,7 @@
                                         {/if}   
                                     {else}
                                         {if condition="$current_user->getId() == $value1->getBuyer() || $current_user->getAdmin()"}
-                                            <a href="?do=confirm_payback&amp;from={$key2}&amp;to={$value1->getBuyer()}&amp;invoice_id={$value1->getId()}&amp;payback_id={$paybacks[$value1->getId()][$key2]->getId()}" title="Confirmer le remboursement">
+                                            <a href="?do=confirm_payback&amp;from={$key2}&amp;to={$value1->getBuyer()}&amp;invoice_id={$value1->getId()}&amp;payback_id={$paybacks[$value1->getId()][$key2]->getId()}&amp;token={$token}" title="Confirmer le remboursement">
                                         {/if}
                                             Reste {$value1->getAmountPerPerson($key2) - $paybacks[$value1->getId()][$key2]->getAmount()}{$currency}
                                         {if condition="$current_user->getId() == $value1->getBuyer() || $current_user->getAdmin()"}
@@ -106,7 +106,7 @@
                     </td>
                     <td>
                         {if condition="$value->getBuyer() == $current_user->getId() || $current_user->getAdmin()"}
-                            <a href="index.php?do=delete_invoice&id={$value->getId()}">Supprimer</a>
+                            <a href="index.php?do=delete_invoice&id={$value->getId()}&amp;token={$token}">Supprimer</a>
                         {else}
                             -
                         {/if}

tpl/default_fr/manage_paybacks.html

@@ -37,6 +37,7 @@
     </fieldset>
     <p>
         <input type="submit" onclick="return confirm('Vous êtes sur le point d\'ajouter un remboursement global incluant les utilisateurs sélectionnés.');" value="Valider"/>
+        <input type="hidden" name="token" value="{$token}"/>
     </p>
     </form>
 {/if}

tpl/default_fr/see_paybacks.html

@@ -8,7 +8,7 @@
                 <dt>{$value->getDate()}</dt>
                 <dd>
                     {if condition="$value->getClosed() !== false"}
-                        <a href="?do=see_paybacks&id={$value->getId()}" onclick="return confirm('Etes-vous sur de vouloir confirmer ce remboursement ? Cette action ne peut etre annulée facilement.');">Remboursement n°{$value->getId()}</a>
+                        <a href="?do=see_paybacks&id={$value->getId()}">Remboursement n°{$value->getId()}</a>
                     {else}
                         [Terminé] Remboursement n°{$value->getId()}
                     {/if}
@@ -40,7 +40,7 @@
                         {else}
                             <td>
                                 {if condition="$table[$key1][$key2] != 0"}
-                                    <a href="?do=confirm_global_paybacks&amp;from={$key1}&amp;to={$key2}&amp;payback_id={$global_paybacks->getId()}" title="Confirmer le remboursement">{$table[$key1][$key2]}</a>
+                                <a href="?do=confirm_global_paybacks&amp;from={$key1}&amp;to={$key2}&amp;payback_id={$global_paybacks->getId()}&amp;token={$token}" title="Confirmer le remboursement" onclick="return confirm('Êtes-vous sûr de vouloir confirmer ce remboursement ? Cette action est irréversible.');">{$table[$key1][$key2]}</a>
                                 {else}
                                     -
                                 {/if}