Opened 9 years ago

Last modified 5 years ago

#1183 closed Bug/Fehler

CSRF-Token Implementation nicht schlüssig — at Version 1

Reported by: anonymous Owned by: somebody
Priority: normal Milestone: modified-shop-2.0.6.0
Component: Admin Version: 2.0.2.2
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by Torsten Riemer)

Bitte dazu meinen Thread im Experten-Forum beachten: CSRF-Token Implementation nicht schlüssig

Wenn 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.
Wenn 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
(nämlich, daß $_POST[$_SESSIONCSRFName] nicht isset ist).
Dasselbe gilt wenn man Ajax-Post benutzt und den Token nicht mit postet weil man keinen form-tag benutzt:

if (is_array($_POST) && count($_POST) > 0) {
  if (isset($_POST[$_SESSION['CSRFName']])) {
    if ($_POST[$_SESSION['CSRFName']] != $_SESSION['CSRFToken']) {
      trigger_error("CSRFToken manipulation.\n".print_r($_POST, true), E_USER_WARNING);
      unset($_POST);
      unset($_GET['action']);
      unset($_GET['saction']);
      
      // create CSRF Token
      $_SESSION['CSRFName'] = xtc_RandomString(6);
      $_SESSION['CSRFToken'] = xtc_RandomString(32);
      if (defined('RUN_MODE_ADMIN')) {
        $messageStack->add(CSRF_TOKEN_MANIPULATION, 'warning');
        $messageStack->add_session(CSRF_TOKEN_MANIPULATION, 'warning');
      }
    }
  } else {
    trigger_error("CSRFToken not defined.\n".print_r($_POST, true), E_USER_WARNING);
    unset($_POST);
    unset($_GET['action']);
    unset($_GET['saction']);
    
    // create CSRF Token
    $_SESSION['CSRFName'] = xtc_RandomString(6);
    $_SESSION['CSRFToken'] = xtc_RandomString(32);
    if (defined('RUN_MODE_ADMIN')) {
      $messageStack->add(CSRF_TOKEN_NOT_DEFINED, 'warning');
      $messageStack->add_session(CSRF_TOKEN_NOT_DEFINED, 'warning');
    }
  }
}

Man müsste das } else { imho ersetzen durch

  } else if($CSRFKeep !== true) { //must ask for $CSRFKeep !== true to really exclude files where xtc_draw_form() is not used, noRiddle
    trigger_error("CSRFToken not defined.\n".print_r($_POST, true), E_USER_WARNING);
    .....

Denn hiermit schließt man erst die Filenames aus dem Array $exclusions aus, im Falle $_POST[$_SESSIONCSRFName] nicht isset ist.

Wahrscheinlich muß noch mehr gemacht werden, denn, wie bereits gesagt wird nirgends abgefragt ob $CSRFKeep auf true gesetzt ist, lediglich indirekt mittels

} elseif ($CSRFKeep === false) {

was als elseif für

if (is_array($_POST) && count($_POST) > 0) {

folgt, was immer zutrifft wenn man etwas per Post absendet.

Es sollte also wohl auch das

if (isset($_POST[$_SESSION['CSRFName']])) {

noch ersetzt werden durch:

if (isset($_POST[$_SESSION['CSRFName']]) && $CSRFKeep === false) {

Der 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.
Mit den von mir o.g. Änderungen funktioniert's bei mir jedenfalls.

Im Forum finden sich auch einige Posts die beschreiben daß das Erweitern des Array $module_exclusions über auto_include() nicht ausreicht um die Fehlermeldung
"CSRF-Token nicht definiert"
zu umgehen.

Gruß,
noRiddle

Change History (1)

comment:1 by Torsten Riemer, 9 years ago

Description: modified (diff)
Milestone: modified-shop-2.0.2.3
Version: 2.0.2.2
Note: See TracTickets for help on using tickets.