ConPresso Community

Fragen, Antworten, Diskussionen rund um das Content Management System ConPresso

 
CPO 4.1.5: CSRF check failed
Gehe zu Seite 1, 2  Weiter
 
Neue Antwort erstellen    ConPresso Community Foren-Übersicht -> Bugs ConPresso 4
Vorheriges Thema anzeigen :: Nächstes Thema anzeigen  
Autor Nachricht
MarkusR
Handbuchversteher


Anmeldungsdatum: 01.01.1970
Beiträge: 6994

BeitragVerfasst am: 21.06.2012 13:22    Titel: CPO 4.1.5: CSRF check failed Antworten mit Zitat

Ja, ich weiß, daß es daß schon als Titel gibt, aber da war es ja kein Bug sondern das korrekte Verhalten.

Jetzt bitte ich aber mal jemanden in einer 4.1.5 folgndes zu testen:

Wenn ich bei "Benutzer" auf "Eigene Daten bearbeiten" gehe und eine Änderung vornehme, dann erhalte ich beim Speichern zuerst
Zitat:
Sicherheitsprüfung fehlgeschlagen. Bitte versuchen Sie es noch einmal.

und bei erneutem Absenden ohne jegliche Änderung oder Korrektur
Zitat:
Der Benutzer LOGIN [NAME] wurde erfolgreich geändert.


Mache ich das Gleiche, wenn ich mich als Benutzer über "Benutzer bearbeiten" aufrufe und bearbeite, sind die Änderungen sofort erfolgreich.

Da wird anscheinend im ersten Fall ein falsches Token benutzt.
Einmal wird ja mittels user_self_modify und dann über user_modify das gleiche Formular aufgerufen, oder?

_________________
Ciao Markus
ConPresso-Module
ConPresso und PHP 7
Nach oben
Benutzer-Profile anzeigen Private Nachricht senden Website dieses Benutzers besuchen
MarkusR
Handbuchversteher


Anmeldungsdatum: 01.01.1970
Beiträge: 6994

BeitragVerfasst am: 23.06.2012 08:09    Titel: Antworten mit Zitat

Ich habe jetzt mal ein wenig im Code gestöbert und habe mal die CSRF-Session-Daten einblenden lassen.

Dabei fielen mir 2 Dinge auf:
1.) Das Ding füllt sich zunehmend
Zitat:
Array
(
[60db3bc02d9fd3ca498eb3def1bad25d] => users_self_save
[6bbf86b9fb5cfbcdc0f94d71389233d6] => setPerPage
[866602bd6f58c0ea1d70f9be4b013e6e] => setPerPage
[26ac0f52335cee36b9364eacb3b9a759] => setPerPage
[ef4866dad12107536f25b90b9d589f41] => setPerPage
[1f19612e3b6bacf47e3ca855b38cf2d0] => setPerPage
[b3e8987327f68709a756fc4e3594b401] => setPerPage
[eca50474cc8aae3a4468997cd44fe1f3] => users_self_save
[b722fb97d17e718b148eabcda47ee469] => users_self_save
[ab7c062bf4b8c0509e1c67a2a5062b3f] => setPerPage
[86171b69942fd1be28e163e49fd7732b] => setPerPage
[2ac2a01fd0fadd2f46182e3123972d46] => setPerPage
[ddfa202e1d64f5c5af2fb078abb91a08] => setPerPage
[565e37ffb2a44967a8eed31604fb1034] => users_self_save
...

2.) Es wird beim Auftreten des Fehlers gleich zweimal ausgegeben, also wird auch zweimal gecheckt!

Den Punkt 1 muß balu mal beleuchten, den Punkt 2 habe ich mir mal angesehen:
in _admin/users,php wird beim Empfang der abgesendeten Daten folgendes ausgeführt
Code:
    case 'users_self_save': // store user data in database {{{
        if (!$antiCSRF->checkToken()) {
            $message[] = ___('Security check failed. Please try again.');
            logaction('placeholders delete', 'CSRF check failed.');
            $views = array('edit_form');
            break;
        }
    case 'users_save':
        if (!$antiCSRF->checkToken()) {
            $message[] = ___('Security check failed. Please try again.');
            if (!empty($_REQUEST['id'])) {
                logaction('edit user', 'CSRF check failed.');
            } else {
                logaction('create user', 'CSRF check failed.');           
            }
            $views = array('edit_form');
            break;
        }

Es passiert also folgendes:
Im Falle von users_self_save wird die Prüfung durchgeführt. Da diese nicht fehlschlägt wird auch nicht abgebrochen sondern weiter ausgeführt. Also kommt anschließend die Prüfung von users_save, die natürlich fehlschlagen muß, weil bei einer erfolgreichen Prüfung der Eintrag aus dem Array entfernt wird, also nicht erneut geprüft werden kann.

Ich habe nun mal probeweise die erste Prüfung komplett entfernt und ... es klappt!
(Die zweite Prüfung findet ja dennnoch statt, also keine Panik Wink )

Code:
    case 'users_self_save': // store user data in database {{{
    case 'users_save':
        if (!$antiCSRF->checkToken()) {
            $message[] = ___('Security check failed. Please try again.');
            if (!empty($_REQUEST['id'])) {
                logaction('edit user', 'CSRF check failed.');
            } else {
                logaction('create user', 'CSRF check failed.');           
            }
            $views = array('edit_form');
            break;
        }

Das mit entfernte
Code:
logaction('placeholders delete', 'CSRF check failed.');

hat da ja eigentlich auch wenig zu suchen... Laughing

_________________
Ciao Markus
ConPresso-Module
ConPresso und PHP 7
Nach oben
Benutzer-Profile anzeigen Private Nachricht senden Website dieses Benutzers besuchen
hscha
ConPresso-Checker


Anmeldungsdatum: 22.02.2006
Beiträge: 496
Wohnort: Berlin

BeitragVerfasst am: 23.06.2012 18:00    Titel: Antworten mit Zitat

Hallo Markus,
ich konnte dieses Verhalten genauso nachvollziehen und habe in admin/users.php die erste Prüfung auskommentiert. Jetzt läufts ohne Fehlermeldung!

Gruß von
Horst

 
Für dieses Posting bedanken sich die folgenden User: MarkusR
Nach oben
Benutzer-Profile anzeigen Private Nachricht senden Website dieses Benutzers besuchen
balu
ConPresso-Entwickler


Anmeldungsdatum: 01.01.1970
Beiträge: 1746

BeitragVerfasst am: 24.06.2012 00:53    Titel: Antworten mit Zitat

Hallo Markus,

zu 1) Das ist richtig. Es gibt immer mal Formulare wie die setPerPage ("Anzahl pro Seite"), die zwar angezeigt, aber nicht abgeschickt und darum auch nicht aus der Session entfernt werden.

Darum wird in anti_csrf.inc.php auch die Anzahl der Tokens in der Session auf 1000 beschränkt (jeweils die ältesten werden gelöscht). So gibt es nicht irgendwann einen Speicherüberlauf wegen zuvieler Tokens.

Ich hatte überlegt, beim Prüfen der Tokens grundsätzlich alle Tokens zu entfernen, weil ja pro Seitenaufruf immer nur eine Aktion durchgeführt werden kann. Wenn man aber mit zwei Tabs arbeitet, gibt es eventuell einen Token für eine zweite ähnliche Aktion. Darum werden alle Token gespeichert.

zu 2) Gut gefunden. Ich hatte da wohl ursprünglich vor, zwei Checks durchzuführen - abhängig von der jeweiligen Aktion. Nachdem ich die Checks eingebaut habe, ist mir aufgefallen, dass $antiCSRF->checkToken(); ja sowieso die Aktion unterscheidet und eins reicht, habe aber die erste Prüfung nicht entfernt.

Das "placeholders delete" zeigt da deutlich, dass ich mir die Stelle Code nicht nochmal genau angeschaut habe. :-/

Balu

_________________
Bartels.Schöne
ConPresso Support & Development
Nach oben
Benutzer-Profile anzeigen Private Nachricht senden
MarkusR
Handbuchversteher


Anmeldungsdatum: 01.01.1970
Beiträge: 6994

BeitragVerfasst am: 24.06.2012 08:38    Titel: Antworten mit Zitat

Mal eine grundsätzliche Frage zur antiCSRF-Funktion...

Beim Betrachten des Codes fällt mir auf, daß nur POST-Variablen berücksichtigt werden, allerdings steht CSRF ja für Cross-Site Request Forgery und betrifft also auch jegliche Requests. Und davon gibt es ja etliche in ConPresso, die GET als Methode benutzen.

Nur mal als Beispiel:
Bei Modulen wird nur die finale Sicherheitsabfrage beim Deinstallieren durch antiCSRF geschützt, weil ein Formular genutzt wird. Alle anderen Aktionen laufen aber über ein GET. Es kann also ein Angreifer nach Lust und Laune Module aktivieren, deaktivieren und auch installieren, wenn sie auf dem Server als Verzeichnis vorhanden sind.

Genausogut könnte er auch Artikel sperren oder freigeben. Und er kann nach Lust und Laune hochgeladene Bilder und Dateien löschen. Und er kann deaktivierte User wiederherstellen.

Ja, mir ist klar, daß das bisher ja sowieso ging und ein Schutz für Formulare besser ist als gar kein Schutz. Aber wenn schon, dann könnte man die Funktion checkToken auf $_REQUEST umbauen.
Die Angabe der Variaben von Hand als Funktionsargumente wäre zwar auch eine Möglichkeit, aber eigentlich müsste die Beschränkung ja nicht sein, oder?

Dann müssten natürlich Aktionen wie das Abschalten eines Moduls mit einem Link in der Form
Code:
../_admin/modules.php?action=modules_deactivate&module=MODULNAME&csrfToken=eca50474cc8aae3a4468997cd44fe1f3

ausgeführt werden, was man mittels angehängtem
Code:
'&csrfToken='.$antiCSRF->getToken('modules_deactivate')
erreichen könnte.
Die Prüfung könnte man derzeit mit einem
Code:
if (!$antiCSRF->checkToken($_REQUEST['action'], $_REQUEST['csrfToken'])) { ...
erzeugen. Schöner wäre es aber, wenn standardmä0g REQUEST statt POST ausgewertet würde und somit auch hier
Code:
if (!$antiCSRF->checkToken()) { ...
reichen würde.
_________________
Ciao Markus
ConPresso-Module
ConPresso und PHP 7
Nach oben
Benutzer-Profile anzeigen Private Nachricht senden Website dieses Benutzers besuchen
balu
ConPresso-Entwickler


Anmeldungsdatum: 01.01.1970
Beiträge: 1746

BeitragVerfasst am: 25.06.2012 09:19    Titel: Antworten mit Zitat

Hi Markus,

das Problem liegt eigentlich an anderer Stelle. Grundsätzlich sollten mit GET keinerlei "ändernde" Aktionen ausgeführt werden.

Siehe auch http://www.w3.org/TR/REC-html40/interact/forms.html#h-17.13.1
Zitat:
The "get" method should be used when the form is idempotent (i.e., causes no side-effects). Many database searches have no visible side-effects and make ideal applications for the "get" method.

If the service associated with the processing of a form causes side effects (for example, if the form modifies a database or subscription to a service), the "post" method should be used.

"Idempotent" in dem Satz bedeutet, dass man einen Request beliebig oft wiederholen kann, ohne dass irgendwelche "Side-Effects" auftreten, also dass keine Änderungen durchgeführt werden.

Nach dem HTML-Standard dürften wir dort also gar keine GET-Anfragen generieren dürfen.

Aber wie Du Dir vorstellen kannst, ist das noch X-mal so viel Aufwand, die auch alle zu ersetzen. Weil der andere Bug aber einen schnellen Release erforderlich gemacht hat, haben wir das erst noch zurückgestellt.

Balu
PS: Siehe auch: http://thinkvitamin.com/code/the-definitive-guide-to-get-vs-post/

_________________
Bartels.Schöne
ConPresso Support & Development
Nach oben
Benutzer-Profile anzeigen Private Nachricht senden
Mr. Magpie
ConPresso-Experte


Anmeldungsdatum: 01.01.1970
Beiträge: 996
Wohnort: Wuppertal

BeitragVerfasst am: 25.06.2012 09:40    Titel: Antworten mit Zitat

Balu, das heißt dann, dass ihr daran schon arbeitet, oder?
_________________
Günther Ludwig, amazingBytes webdesign   

Referenzen finden Sie hier: amazingBytes webdesign - Referenzen
Nach oben
Benutzer-Profile anzeigen Private Nachricht senden E-Mail senden Website dieses Benutzers besuchen
MarkusR
Handbuchversteher


Anmeldungsdatum: 01.01.1970
Beiträge: 6994

BeitragVerfasst am: 25.06.2012 09:48    Titel: Antworten mit Zitat

Zitat:
Aber wie Du Dir vorstellen kannst, ist das noch X-mal so viel Aufwand, die auch alle zu ersetzen.

Da habe ich auch einige Zeit (nach meinem Posting) drüber nachgedacht. Auch wenn es grundsätzlich möglich wäre, ist es wenig praktikabel, weil auf jeder Seite Dutzende ungenutzte Tokens entstehen würden und damit die gesetzte 1000er-Grenze rasend schnell erreicht wäre. Wehe, wenn man dann in der Entwicklung mal ein print_r($_SESSION) einbaut Twisted Evil

Mittlerweile glaube ich, daß es - zumindest in einem komplexen CMS - keinen wirklichen Schutz dagegen gibt, außer eben nicht zu viele Webseiten parallel zur Arbeit im CMS zu nutzen.
Viel besser ist natürlich: Backup, Backup, Backup! Nach Murphy's Law knallt es ja doch irgendwann.
Und wenn sich halt doch ein aufsässiger Redakteur irgendwo ein kleines "automatisches Reaktivieren" in einen Artikel einsetzt, dann hat er ja noch keine Rechte.

Wobei mir ein weiterer "Bug" auffällt:
Wenn ich ein Mitglied reaktiviere (das ja bekanntlich keine Rechte hat und weshalb ja auch der Rechte-Button inaktiv ist) dann werde ich dennoch zur Rechte-Vergabe über einen einblendeten Link aufgefordert. Man kann ihm dann auch Rechte geben und diese erfolgreich abspeichern.
Ist zwar (vermutlich) nur ein kosmetisches Problem, aber vielleicht kann man das ja noch korrigieren (entweder kein Link oder bei der Rechtevergabe erst noch prüfen, wem man da gerade Rechte gibt...)

_________________
Ciao Markus
ConPresso-Module
ConPresso und PHP 7
Nach oben
Benutzer-Profile anzeigen Private Nachricht senden Website dieses Benutzers besuchen
balu
ConPresso-Entwickler


Anmeldungsdatum: 01.01.1970
Beiträge: 1746

BeitragVerfasst am: 26.06.2012 09:01    Titel: Antworten mit Zitat

Mr. Magpie hat Folgendes geschrieben:
Balu, das heißt dann, dass ihr daran schon arbeitet, oder?


Das heißt, dass es auf unserer Liste steht und wir über eine Umsetzung nachdenken. Die Frage ist, ob sich der Aufwand im Vergleich zum "Sicherheitsgewinn" in diesem Fall lohnt.

Wie Markus schon richtig geschrieben hat, ist das auch ein Performance- und Speicherproblem.

Auf der Artikelliste gibt es pro Artikel ungefähr 14 Aktionen, die durchgeführt werden können und Änderungen erzeugen. Sortieren, Bearbeiten, Löschen, Freigeben, usw. Alle diese Aktionen können vom User rückgängig gemacht werden.

Da reicht es schon, sich 65 Artikel auf einer Seite anzeigen zu lassen und der CSRF-Speicher ist aufgebraucht. Man könnte den vergrößern, aber das muss auch alles in jeder Session gespeichert und bei jeden Seitenaufruf geladen und wieder gespeichert werden.

Und dann ist da die Frage, wie wahrscheinlich es ist, dass ein Hacker einen Artikel ohne Wissen des Redakteurs blocken oder umsortieren will. Wir reden hier nicht vom Editieren - das ist ja bereits geschützt.

Wir müssen schauen, ob und wie wir das machen können.

Balu

_________________
Bartels.Schöne
ConPresso Support & Development

 
Für dieses Posting bedanken sich die folgenden User: Mr. Magpie
Nach oben
Benutzer-Profile anzeigen Private Nachricht senden
Mr. Magpie
ConPresso-Experte


Anmeldungsdatum: 01.01.1970
Beiträge: 996
Wohnort: Wuppertal

BeitragVerfasst am: 26.06.2012 09:20    Titel: Antworten mit Zitat

Ich dachte ja auch eher an so Dinge wie aktivieren/deaktivieren/deinstallieren von Modulen.
_________________
Günther Ludwig, amazingBytes webdesign   

Referenzen finden Sie hier: amazingBytes webdesign - Referenzen
Nach oben
Benutzer-Profile anzeigen Private Nachricht senden E-Mail senden Website dieses Benutzers besuchen
dutch
ConPresso-Experte


Anmeldungsdatum: 04.07.2007
Beiträge: 515

BeitragVerfasst am: 23.10.2012 16:29    Titel: Antworten mit Zitat

Hallo zusammen,

ich habe gerade 4.1.5 installiert und bekomme die Meldung mit dem misglückten Sicherheits-check jedesmal, wenn ich das backend direkt aufrufe über ....../_admin/index.php
(Bei den bisherigen cpo Versionen klappte das immer so...)

Wenn ich dann im Menü auf "login" klicke und es nochmal versuche, klappt es mit der Anmeldung.

Ist das jetzt ein normales Verhalten und falls ja, wie rufe ich dann korrekt die backend-login Seite auf?

Grüße,
dutch


Nachtrag:
Habe eben nochmal getestet mit anderen Browsern. Der backend-login klappt ohne Probleme beim IE8. Die Sicherheits-check Meldung kommt aber bei Firefox, Chrome und Safari.
Nach oben
Benutzer-Profile anzeigen Private Nachricht senden
MarkusR
Handbuchversteher


Anmeldungsdatum: 01.01.1970
Beiträge: 6994

BeitragVerfasst am: 23.10.2012 17:58    Titel: Antworten mit Zitat

Prüfe mal, ob Du Dich schon mit der richtigen Domain anmeldest.

http://www.domain.de ist nicht zwingend das gleiche wie http://domain.de

Findet im Anmeldeprozess ein Wechsel statt?

_________________
Ciao Markus
ConPresso-Module
ConPresso und PHP 7
Nach oben
Benutzer-Profile anzeigen Private Nachricht senden Website dieses Benutzers besuchen
dutch
ConPresso-Experte


Anmeldungsdatum: 04.07.2007
Beiträge: 515

BeitragVerfasst am: 23.10.2012 19:05    Titel: Antworten mit Zitat

Nein, es läuft immer über www.domain....
Auch wenn ich das www weglasse, springt die Adresszeile sofort auf www.domain....

Habe es jetzt schon sehr oft getestet aber es bleibt merkwürdig. Wenn die Anmeldung dann funktioniert hat (über den Umweg des "login" buttons), kann ich mich problemlos wieder abmelden und ohne Probleme auch wieder anmelden, nur klappt es eben nicht beim ersten Mal.
Das Problem taucht auch (meistens) dann wieder auf, wenn man die login-Seite neu lädt im Browser...


Gruß,
dutch
Nach oben
Benutzer-Profile anzeigen Private Nachricht senden
balu
ConPresso-Entwickler


Anmeldungsdatum: 01.01.1970
Beiträge: 1746

BeitragVerfasst am: 03.11.2012 13:22    Titel: Antworten mit Zitat

Hallo,

das kann eigentlich nur mit Cookies / Sesssion-Problemen zusammen hängen. In der Session wird für jedes Formular ein Sicherheitscode gespeichert, der dann beim Übertragen des Formulars wieder geprüft wird.

Eventuell gibt es irgendein Plugin oder ähnliches, was Schabernack mit den Cookies im Browser macht und diese beim ersten Aufruf einer Seite nicht akzeptiert oder mitsendet?

Balu

_________________
Bartels.Schöne
ConPresso Support & Development
Nach oben
Benutzer-Profile anzeigen Private Nachricht senden
dutch
ConPresso-Experte


Anmeldungsdatum: 04.07.2007
Beiträge: 515

BeitragVerfasst am: 29.04.2013 13:36    Titel: Antworten mit Zitat

Hallo,

es ist wieder eine Weile her, aber das Problem mit der Sicherheitscheck-Meldung taucht bei mir wieder (immer noch) auf.

Das letzte Mal kam es immer bei der Anmeldung im backend (mit einer 4.1.5). Dort taucht die Meldung jetzt nur noch ab und zu auf und ist zu verschmerzen.

Jetzt habe ich aber eine Seite mit cpo 4.1.6 erstellt, inkl. frontend login, und dort erscheint die Meldung bei jedem Loginversuch...

Wenn ich dann im Meldungfenster den Login wiederhole, bin ich eingeloggt.

Kann man das irgendwie beheben?

Gruß,
dutch
Nach oben
Benutzer-Profile anzeigen Private Nachricht senden
Beiträge der letzten Zeit anzeigen:   
Neue Antwort erstellen    ConPresso Community Foren-Übersicht -> Bugs ConPresso 4 Alle Zeiten sind GMT + 1 Stunde
Gehe zu Seite 1, 2  Weiter
Seite 1 von 2

 
Gehe zu:  
Du kannst keine Beiträge in dieses Forum schreiben.
Du kannst auf Beiträge in diesem Forum nicht antworten.
Du kannst deine Beiträge in diesem Forum nicht bearbeiten.
Du kannst deine Beiträge in diesem Forum nicht löschen.
Du kannst an Umfragen in diesem Forum nicht mitmachen.
Du kannst Dateien in diesem Forum nicht posten
Du kannst Dateien in diesem Forum herunterladen