Changes between Initial Version and Version 1 of Ticket #1183


Ignore:
Timestamp:
May 2, 2017, 1:14:10 PM (9 years ago)
Author:
Torsten Riemer
Comment:

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #1183

    • Property Milestonemodified-shop-2.0.2.3
    • Property Version2.0.2.2
  • Ticket #1183 – Description

    initial v1  
    1 Bitte dazu [https://www.modified-shop.org/forum/index.php?topic=37151 meinen Thread im Experten-Forum] beachten.
     1Bitte dazu meinen Thread im Experten-Forum beachten: [https://www.modified-shop.org/forum/index.php?topic=37151.0 CSRF-Token Implementation nicht schlüssig]
     2
     3Wenn man sich den Code in der ''/inc/csrf_token.inc.php'' anschaut, so wird bei aufgerufenen Filenames die sich im Array ''$module_exclusions'' und somit im Array ''$exclusions'' befinden die Variable ''$CSRFKeep'' auf ''true'' gesetzt. Es wird jedoch nirgends abgefragt was passieren soll wenn sie auf ''true'' steht.
     4Wenn man nun in einem Script '''nicht die Funktion ''xtc_draw_form()'' benutzt''' wird der '''Token ja nicht in einem hidden-field implementiert''' und es trifft dann der Teil des ''else'' in folgendem Code zu
     5(nämlich, daß ''$_POST[$_SESSION['CSRFName']]'' nicht ''isset'' ist).
     6Dasselbe gilt wenn man Ajax-Post benutzt und den Token nicht mit postet weil man keinen form-tag benutzt:
     7{{{
     8if (is_array($_POST) && count($_POST) > 0) {
     9  if (isset($_POST[$_SESSION['CSRFName']])) {
     10    if ($_POST[$_SESSION['CSRFName']] != $_SESSION['CSRFToken']) {
     11      trigger_error("CSRFToken manipulation.\n".print_r($_POST, true), E_USER_WARNING);
     12      unset($_POST);
     13      unset($_GET['action']);
     14      unset($_GET['saction']);
     15     
     16      // create CSRF Token
     17      $_SESSION['CSRFName'] = xtc_RandomString(6);
     18      $_SESSION['CSRFToken'] = xtc_RandomString(32);
     19      if (defined('RUN_MODE_ADMIN')) {
     20        $messageStack->add(CSRF_TOKEN_MANIPULATION, 'warning');
     21        $messageStack->add_session(CSRF_TOKEN_MANIPULATION, 'warning');
     22      }
     23    }
     24  } else {
     25    trigger_error("CSRFToken not defined.\n".print_r($_POST, true), E_USER_WARNING);
     26    unset($_POST);
     27    unset($_GET['action']);
     28    unset($_GET['saction']);
     29   
     30    // create CSRF Token
     31    $_SESSION['CSRFName'] = xtc_RandomString(6);
     32    $_SESSION['CSRFToken'] = xtc_RandomString(32);
     33    if (defined('RUN_MODE_ADMIN')) {
     34      $messageStack->add(CSRF_TOKEN_NOT_DEFINED, 'warning');
     35      $messageStack->add_session(CSRF_TOKEN_NOT_DEFINED, 'warning');
     36    }
     37  }
     38}
     39}}}
     40Man müsste das ''} else {'' imho ersetzen durch
     41{{{
     42  } else if($CSRFKeep !== true) { //must ask for $CSRFKeep !== true to really exclude files where xtc_draw_form() is not used, noRiddle
     43    trigger_error("CSRFToken not defined.\n".print_r($_POST, true), E_USER_WARNING);
     44    .....
     45}}}
     46Denn hiermit schließt man erst die Filenames aus dem Array ''$exclusions'' aus, im Falle ''$_POST[$_SESSION['CSRFName']]'' nicht ''isset'' ist.
     47
     48Wahrscheinlich muß noch mehr gemacht werden, denn, wie bereits gesagt wird nirgends abgefragt ob ''$CSRFKeep'' auf ''true'' gesetzt ist, lediglich indirekt mittels
     49{{{
     50} elseif ($CSRFKeep === false) {
     51}}}
     52was als ''elseif'' für
     53{{{
     54if (is_array($_POST) && count($_POST) > 0) {
     55}}}
     56folgt, was immer zutrifft wenn man etwas per Post absendet.
     57
     58Es sollte also wohl auch das
     59{{{
     60if (isset($_POST[$_SESSION['CSRFName']])) {
     61}}}
     62noch ersetzt werden durch:
     63{{{
     64if (isset($_POST[$_SESSION['CSRFName']]) && $CSRFKeep === false) {
     65}}}
     66Der ganze Logik-Aufbau der Datei ist imho ein wenig "suspekt", da schwer zu durchschauen ist wan was genau passiert mit den ganzen ''if elseif else'' -Konstruktionen, weshalb ich's auch noch nicht ganz durchschaut habe.
     67Mit den von mir o.g. Änderungen funktioniert's bei mir jedenfalls.
     68
     69Im Forum finden sich auch einige Posts die beschreiben daß das Erweitern des Array ''$module_exclusions'' über ''auto_include()'' nicht ausreicht um die Fehlermeldung
     70"''CSRF-Token nicht definiert''"
     71zu umgehen.
    272
    373Gruß,