Neuigkeiten
  • Die modified eCommerce Shopsoftware ist kostenlos, aber nicht umsonst.
  • Damit wir die modified eCommerce Shopsoftware auch zukünftig kostenlos anbieten können:

Autor Thema: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)  (Gelesen 50423 mal)

Offline Tomcraft

  • modified Team
  • *****
  • Gravatar
  • Beiträge: 43.519
  • Geschlecht: Männlich
    • Teile Beitrag
    • https://www.modified-shop.org
Die Installation des folgenden Sicherheitspatches wird allen Shopbetreibern für alle Shopversionen empfohlen. Unter bestimmten Voraussetzungen konnte ein Gast die Admin-Box sehen und so die Anzahl der Kunden & Bestellungen eines Shops ersehen (Ein Zugriff auf den Adminbereich war jedoch zu keinem Zeitpunkt möglich!).

Vielen Dank an dieser Stelle an Karsten Geyer von Fishnet® Services für den Hinweis auf diese Sicherheitslücke.

Download des Fixes: Klick mich

Wir wünschen euch weiterhin gute Geschäfte und viel Spass mit unserer Shopsoftware.

Euer modified eCommerce Shopsoftware Team

Linkback: https://www.modified-shop.org/forum/index.php?topic=31795.0

Offline noRiddle

  • Experte
  • *****
  • Beiträge: 9.736
  • Geschlecht: Männlich
    • Teile Beitrag
    • Webdesign Bonn - Köln
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #1 am: 17. Dezember 2014, 16:25:57 »
Vielen Dank,
auch an fischnet.

In der /templates/YOUR_TEMPLATE/source/boxes.php sollte man auch noch hier
Code: PHP  [Auswählen]
if ($_SESSION['customers_status']['customers_status_id'] == 0) {
    require_once(DIR_WS_BOXES . 'admin.php');
    $smarty->assign('is_admin', true);
}

die 0 quote-n damit sie nicht als Boolean gewertet werden kann.
Code: PHP  [Auswählen]
if ($_SESSION['customers_status']['customers_status_id'] == '0') {
    require_once(DIR_WS_BOXES . 'admin.php');
    $smarty->assign('is_admin', true);
}

Die nicht ge-quote-te 0 bei einer Abfrage nach $_SESSION['customers_status']['customers_status_id'] kommt übrigens noch öfter Im Core-Code vor.
Z.B. in
  • /account_edit.php
  • /inc/xtc_db_error.inc.php
  • /includes/functions/sessions.php
  • /shoproot/includes/header.php

Wer weiß was da sonst noch alles an Fallen drinstecken könnte.

Gruß,
noRiddle

Offline golferteddy

  • Schreiberling
  • ****
  • Beiträge: 368
  • Geschlecht: Männlich
    • Teile Beitrag
    • Teddy-Fabrik - Der offizielle eshop der HERMANN-Spielwaren GmbH
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #2 am: 17. Dezember 2014, 17:23:10 »
Danke an fischnet und noRiddle

Frohe Weihanchten

Offline Bonsai

  • Viel Schreiber
  • *****
  • Beiträge: 4.147
  • Geschlecht: Männlich
    • Teile Beitrag
    • J.K.Fischer Shop
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #3 am: 17. Dezember 2014, 17:36:24 »
Besten Dank!

Offline ralph_84

  • Fördermitglied
  • *****
  • Beiträge: 499
  • Geschlecht: Männlich
    • Teile Beitrag
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #4 am: 17. Dezember 2014, 17:46:18 »
Auch von mir ein Herzliches Danke

 :thx:

an fischnet und noRiddle

Offline ado

  • Fördermitglied
  • *****
  • Beiträge: 195
    • Teile Beitrag
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #5 am: 17. Dezember 2014, 17:47:28 »
ich auch dickes DANKE!

LG
ado

Offline voodoopupp

  • Fördermitglied
  • *****
  • Beiträge: 1.337
    • Teile Beitrag
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #6 am: 17. Dezember 2014, 17:49:12 »
Danke auch von mir.

Interessant wäre zu wissen, wie es um die Dinge steht, die noRiddle angesprochen hat.

Offline h-h-h

  • modified Team
  • *****
  • Beiträge: 4.563
    • Teile Beitrag
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #7 am: 17. Dezember 2014, 18:15:33 »
Mein Freund, noRiddle:

Code: PHP  [Auswählen]
echo '0' == 0 ? 'true' : 'false';  #> true

echo '0' === 0 ? 'true' : 'false'; #> false

Dein Code ergibt so für mich keinen Sinn, du meinst vermutlich ===.
Und ich wüsste gerne auch in welchem Fall das eintreten könnte.

Schreib mich einfach per Mail an, bevor du hier einige verunsicherst.

LG, h-h-h

Offline brotherlui

  • Neu im Forum
  • *
  • Beiträge: 43
    • Teile Beitrag
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #8 am: 17. Dezember 2014, 19:11:19 »
hmmm ...

Zitat
Deine Code ergibt so für mich keinen Sinn, du meinst vermutlich ===.
Und ich wüsste gerne auch in welchem Fall das eintreten könnte....

*totalverunsichertnun* ja was jetzt nun ???

Offline h-h-h

  • modified Team
  • *****
  • Beiträge: 4.563
    • Teile Beitrag
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #9 am: 17. Dezember 2014, 19:17:47 »
Keine Sorge, alles ist gut.  :-)
Werde mit noRiddle telefonieren.

LG, h-h-h

Offline Mantronix

  • Fördermitglied
  • *****
  • Beiträge: 146
    • Teile Beitrag
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #10 am: 18. Dezember 2014, 01:04:12 »
Von mir auch

:thx:

an fischnet und noRiddle

Offline noRiddle

  • Experte
  • *****
  • Beiträge: 9.736
  • Geschlecht: Männlich
    • Teile Beitrag
    • Webdesign Bonn - Köln
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #11 am: 18. Dezember 2014, 02:46:42 »
Vorab:
Warum bedankt Ihr euch bei mir ?
fishnet war der der zuerst einen Fix vorgeschlagen hat.
In privatem Mail-Verkehr war ich allerdings auch schon für den nun hier vorgestellten offiziellen Fix für die /includes/write_customers_status.php.

Meinen Zusatz finde ich allerdings begründet.

Mein Freund, noRiddle:

Code: PHP  [Auswählen]
echo '0' == 0 ? 'true' : 'false';  #> true

echo '0' === 0 ? 'true' : 'false'; #> false

Deine Code ergibt so für mich keinen Sinn, du meinst vermutlich ===.
Und ich wüsste gerne auch in welchem Fall das eintreten könnte.

Schreib mich einfach per Mail an, bevor du hier einige verunsicherst.

LG, h-h-h

"Keinen Sinn" finde ich ein wenig übertrieben ;-).
Deine codierte Fragestellung ist nicht zielführend, sie müsste anders lauten,
denn,
da mysql_fetch_array() false ergibt wenn kein Ergebnis der Query vorhanden ist, vermutete ich, daß dann die $_SESSION['customers_status']['customers_status_id'] in der /includes/write_customers_status.php auf false gesetzt würde (siehe Code dort) und false == 0 als Boolean.
Der Test müsste also so aussehen, dachte ich:
Code: PHP  [Auswählen]
echo '<h1>noRiddle</h1>';
echo '0' == true ? 'true<br />' : 'false<br />'; #>false
echo '0' == false ? 'true<br />' : 'false<br />'; #>true
echo 0 == false ? 'true<br />' : 'false<br />'; #>true
echo 0 === false ? 'true<br />' : 'false<br />'; #>false
echo false == 0 ? 'true<br />' : 'false<br />'; #>true // <= das ist der wichtigste Test
echo false === 0 ? 'true<br />' : 'false<br />'; #>false // <= das ist der zweitwichtigste Test
echo true == 1 ? 'true<br />' : 'false<br />'; #>true

Nachdem ich mit h-h-h alles durchgetestet habe kam raus, daß die $_SESSION['customers_status']['customers_status_id'] auf NULL gesetzt wird wenn in der Query $customers_status_query_1 kein Ergebnis gefunden wurde.
Der Test sollte also so aussehen:
Code: PHP  [Auswählen]
echo '0' == NULL ? 'true<br />' : 'false<br />';  #> false
echo 0 == NULL ? 'true<br />' : 'false<br />'; #> true

Wenn also in der von mir zitierten /templates/YOUR_TEMPLATE/source/boxes.php die Abfrage so lautet (also nicht auf String 0 geprüft):
Code: PHP  [Auswählen]
    if ($_SESSION['customers_status']['customers_status_id'] == 0) {
        require_once(DIR_WS_BOXES . 'admin.php');
        $smarty->assign('is_admin', true);
    }
ist das verkehrt.

Damit man aber nicht immer und überall darauf achten muß ist der Fix in der /includes/write_customers_status.php den das Team zur Verfügung gestellt hat genau richtig, nämlich an der Wurzel gepackt.

Also, alles gut, auch ohne meinen Fix in der /templates/YOUR_TEMPLATE/source/boxes.php, auch wenn er korrekter wäre als nicht auf String zu testen.

Gruß und good night,
noRiddle

Offline web4design

  • Experte
  • *****
  • Beiträge: 1.068
    • Teile Beitrag
    • http://www.web4design.de
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #12 am: 18. Dezember 2014, 08:42:06 »
:thx: @all ;)

Offline Fakrae

  • Viel Schreiber
  • *****
  • Beiträge: 997
    • Teile Beitrag
    • Futterkrämerei
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #13 am: 18. Dezember 2014, 08:47:17 »
Danke!

Offline OM-D

  • Neu im Forum
  • *
  • Beiträge: 40
    • Teile Beitrag
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #14 am: 18. Dezember 2014, 09:37:11 »
Vielen Dank für den fix!

Offline andrewx

  • Fördermitglied
  • *****
  • Beiträge: 12
  • Geschlecht: Männlich
    • Teile Beitrag
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #15 am: 18. Dezember 2014, 09:53:13 »
Vielen Dank allen für den unermüdlichen Einsatz!

Offline karl

  • Schreiberling
  • ****
  • Beiträge: 439
    • Teile Beitrag
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #16 am: 18. Dezember 2014, 14:19:21 »
Dank auch von mir!

Und wer hat in der anderen Streitfrage nun Recht? noRiddle oder h-h-h ?

Offline fishnet

  • Fördermitglied
  • *****
  • Beiträge: 4.601
  • Geschlecht: Männlich
    • Teile Beitrag
    • Fishnet Services
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #17 am: 18. Dezember 2014, 14:30:28 »
Das ist letztendlich egal, weil du die Lösung vom Modified Team (Post 1) nehmen solltest, die zwar nicht die performanteste aber die sicherste (doppelt gemoppelt) ist.
Ich habe auch noch eine Frage, ab wann wird sich der Fix auch im Download des Shops finden ?

Offline web0null

  • Experte
  • *****
  • Beiträge: 1.998
    • Teile Beitrag
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #18 am: 18. Dezember 2014, 14:38:11 »
Ich habe mir das jetzt nicht "ganz genau" angesehen, aber mir stellt sich eigentlich die Frage, bzw. Überlegung,
... sollte man nicht einfach dafür sorgen dass $_SESSION['customer_id'] richtig bzw. überhaupt gefüllt wird, damit man nicht später solche Verrenkungen machen muss.

Ich meine fishnet´s Lösung ist sicher nicht verkehrt, aber das "Problem" sollte doch am Uhrsprung gelöst werden.

Oder habe ich da etwas falsch verstanden?
Gruß

Offline web0null

  • Experte
  • *****
  • Beiträge: 1.998
    • Teile Beitrag
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #19 am: 18. Dezember 2014, 15:29:05 »
...soll natürlich "Ursprung" heißen.

Offline webald

  • modified Team
  • *****
  • Beiträge: 2.701
    • Teile Beitrag
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #20 am: 18. Dezember 2014, 15:41:21 »
Deshalb hatte ich im anderen Post auch geschrieben, dass die Admingruppe im Standard nicht 0 sonder etwas anderes etwa -1 sein sollte.

Offline web0null

  • Experte
  • *****
  • Beiträge: 1.998
    • Teile Beitrag
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #21 am: 18. Dezember 2014, 15:51:25 »
Ja ich weiß was du meinst, hat aber nur indirekt etwas damit zu tun.
Fakt ist, dass die $_SESSION['customer_id'] falsch gesetzt wird, und das ist dass Problem.
Denn das falsche befüllen bzw. setzen der $_SESSION['customer_id'] ist der Ursprung des Fehlers, und nur da sollte man eingreifen.
Alles andere ist nur "vorhandene Fehler im Nachhinein wieder umzubiegen".

Aber ich "glaube" schon zu wissen warum die Umsetzung so erfolgt ist, wie sie im "FIX" ist.  ;-)

Gruß

Offline pl1

  • Neu im Forum
  • *
  • Beiträge: 31
    • Teile Beitrag
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #22 am: 18. Dezember 2014, 17:21:49 »
... sollte man nicht einfach dafür sorgen dass $_SESSION['customer_id'] richtig bzw. überhaupt gefüllt wird, damit man nicht später solche Verrenkungen machen muss.

Ja, und ließe sich nicht die gesamte includes/write_customers_status.php vereinfachen?

Also sowas in der Art (gedanklich, ungetestet!)
Code: PHP  [Auswählen]
if(isset($_SESSION['customer_id'])) {
  $customers_status_query = xtc_db_query("SELECT cs.* FROM ".TABLE_CUSTOMERS." c
    JOIN "
.TABLE_CUSTOMERS_STATUS." cs
    ON cs.customers_status_id=c.customers_status
    WHERE c.customers_id = '"
.$_SESSION['customer_id']."'
    AND cs.language_id = '"
.$_SESSION['languages_id']);
  if(xtc_db_num_rows($customers_status_query_1) > 0) {
    $_SESSION['customers_status']= xtc_db_fetch_array($customers_status_query);
  }else{
    unset($_SESSION['customer_id']);
    xtc_redirect(xtc_href_link(FILENAME_LOGOFF));
  }
}else{
  $customers_status_query = xtc_db_query("
    SELECT * FROM "
.TABLE_CUSTOMERS_STATUS."
    WHERE customers_status_id = '"
.DEFAULT_CUSTOMERS_STATUS_ID_GUEST."'
    AND language_id = '"
. $_SESSION['languages_id'] . "'");
  $_SESSION['customers_status']= xtc_db_fetch_array($customers_status_query);
}

Der Dateiname "write_customers_status" suggeriert etwas anderes als dann in der Datei wirklich gemacht wird. Es werden ja nur die passenden Werte aus der Datenbanktabelle customers_status in die $SESSION mit geladen.

Und gerade sehe ich, dass includes/write_customers_status.php ähnlich ist mit
inc/xtc_get_customer_status_value.inc.php

Dort wird in die $_SESSION geschrieben
Code: PHP  [Auswählen]
function xtc_get_customer_status_value($customer_id) {
....
# XXX Setzt! Sessionstatus des aktuellen Nutzers auf den Status des angefragen Nutzer mit customer_id! Unerwartetes Verhalten IMHO
$_SESSION['customer_status_value'] = $customer_status_value;
return $customer_status_value;
}
Allerdings scheint das toter Code zu sein, denn xtc_get_customer_status_value wird nirgends aufgerufen
Code: PHP  [Auswählen]
grep -r xtc_get_customer_status_value
xtc_get_customer_status fände ich passender als xtc_get_customer_status_value. Diese Funktion gibt es im admin-Bereich in der admin/includes/general.php

Man beachte aber den Unterschied zwischen JOIN und LEFT JOIN.

Offline noRiddle

  • Experte
  • *****
  • Beiträge: 9.736
  • Geschlecht: Männlich
    • Teile Beitrag
    • Webdesign Bonn - Köln
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #23 am: 18. Dezember 2014, 18:10:19 »
Dank auch von mir!

Und wer hat in der anderen Streitfrage nun Recht? noRiddle oder h-h-h ?

Das haben h-h-h und meine Wenigkeit doch geklärt und ich habe es hier ausführlich erklärt.
Ich hatte Recht, allerdings mit der verkehrten Begründung, da $_SESSION['customers_status']['customers_status_id'] ohne den Fix auf die Konstante NULL gesetzt wurde und nicht auf Boolean 0 (== false).

Im übrigen hat fishnet Recht mit seiner Antwort, es ist egal da der hier vorgestellte Fix an der Wurzel angreift.

Ich habe mir das jetzt nicht "ganz genau" angesehen, aber mir stellt sich eigentlich die Frage, bzw. Überlegung,
... sollte man nicht einfach dafür sorgen dass $_SESSION['customer_id'] richtig bzw. überhaupt gefüllt wird, damit man nicht später solche Verrenkungen machen muss.

Ich meine fishnet´s Lösung ist sicher nicht verkehrt, aber das "Problem" sollte doch am Uhrsprung gelöst werden.

Oder habe ich da etwas falsch verstanden?
Gruß
Lieber web0null.
  • ist das Problem ja am Ursprung/an der Wurzel gepackt indem die
    $_SESSION['customers_status']['customers_status_id']
    durch den Fix nicht mehr auf 0, false oder NULL gesetzt wird wenn die Query in der /includes/write_customers_status.php kein Result liefert
    und
  • geht es nicht um die $_SESSION['customer_id'] (= customers_id aus der Tabelle customers)
    - die ist nämlich immer gesetzt sobald jemand eingeloggt ist -
    sondern um die
    $_SESSION['customers_status']['customers_status_id'] (= customers_status aus der Tabelle customers)
    die in besagter Datei gefüllt bzw. definiert wird.

Deshalb hatte ich im anderen Post auch geschrieben, dass die Admingruppe im Standard nicht 0 sonder etwas anderes etwa -1 sein sollte.

In der Tat wäre es von vornherein klüger für den customers_status keine Werte zu benutzen die als Boolean mißverstanden werden könnten, also weder 1 noch 0 (vielleicht gar überhaupt keine numerischen Werte), damit man beim Codieren nicht ständig aufpassen muß.
Da das allerdings noch eine sog. Altlast ist müsste man zuviel Code anfassen wollte man das ändern.
Man sollte halt immer so programmieren, daß man an der Wurzel Fehler verhindert und das ist mit dem vorliegenden Fix geschehen.
Niemand hat geahnt, daß ein leeres Result aus einer Query die mittels mysql_fetch_array() in ein Array geschrieben wird einen Wert von NULL ausgibt wenn er in eine Session geschrieben wird.
Ich hätte gedacht, daß es false ergeben müsste, da das PHP-Manual sagt, daß mysql_fetch_array() false ausgibt wenn keine Row gefunden wurde. Im Manual steht zwar auch dies:
Zitat:
"Diese Funktion setzt NULL-Felder auf den PHP Wert-NULL."
aber was ist ein NULL-Feld ?, ich verstehe es so, daß es ein Feld ist dessen Inhalt NULL ist, wenn man jedoch gar kein Result hat weil es keinen Eintrag gibt ?

Gruß,
noRiddle

Offline web0null

  • Experte
  • *****
  • Beiträge: 1.998
    • Teile Beitrag
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #24 am: 18. Dezember 2014, 18:35:45 »
Lieber noRiddle,
Zitat
ist das Problem ja am Ursprung/an der Wurzel gepackt
Sehe ich anders.
Denn anscheinend ist ja in $_SESSION['customer_id']  "ein Wert" drinnen womit in weiterer folge hiermit,
Zitat
$customers_status_query_1 = xtc_db_query("SELECT customers_status FROM " . TABLE_CUSTOMERS . " WHERE customers_id = '" . $_SESSION['customer_id'] . "'");

nichts raus kommt.

Also ist meiner bescheidenen Meinung nach, die Wurzel, eben das richtige befüllen der $_SESSION['customer_id'].

Denn wenn man dort schon einen Wert hinterlegt der auch in der TABLE_CUSTOMERS zu finden ist, hätte man das Problem eines leeren Resultats nicht.

Eigentlich kann die $_SESSION['customer_id'] nur folgende "Fehler" aufweisen,
  • sie ist gesetzt und leer
  • oder gesetzt und mit der Zahl 0 gefüllt
  • oder gesetzt und mit einem falschen bzw. alten Wert gefüllt

Und alle 3 Möglichkeiten wären falsch und somit schon vor dem setzen von $_SESSION['customer_id'], abzufangen.

Gruß

Offline MW

  • Fördermitglied
  • *****
  • Beiträge: 421
  • Geschlecht: Männlich
    • Teile Beitrag
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #25 am: 18. Dezember 2014, 19:34:52 »
Danke an fishnet, noRiddle  :thx:

und web0null das du mit gegrübelt hast  ;-)

Offline Tomcraft

  • modified Team
  • *****
  • Gravatar
  • Beiträge: 43.519
  • Geschlecht: Männlich
    • Teile Beitrag
    • https://www.modified-shop.org
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #26 am: 18. Dezember 2014, 19:48:38 »
Ich habe soeben Beiträge aus dem Thema gelöscht, die quasi eine Anleitung zur Ausnutzung des Fehlers liefern.

Grüße

Torsten

Offline innuXTC

  • Schreiberling
  • ****
  • Beiträge: 474
    • Teile Beitrag
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #27 am: 18. Dezember 2014, 19:55:42 »
Danke Torsten, ... dann sind wir ja einer Meinung :)

Offline h-h-h

  • modified Team
  • *****
  • Beiträge: 4.563
    • Teile Beitrag
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #28 am: 18. Dezember 2014, 20:31:06 »
Ja, und ließe sich nicht die gesamte includes/write_customers_status.php vereinfachen?

Also sowas in der Art (gedanklich, ungetestet!)
Code: PHP  [Auswählen]
if(isset($_SESSION['customer_id'])) {
  $customers_status_query = xtc_db_query("SELECT cs.* FROM ".TABLE_CUSTOMERS." c
    JOIN "
.TABLE_CUSTOMERS_STATUS." cs
    ON cs.customers_status_id=c.customers_status
    WHERE c.customers_id = '"
.$_SESSION['customer_id']."'
    AND cs.language_id = '"
.$_SESSION['languages_id']);
  if(xtc_db_num_rows($customers_status_query_1) > 0) {
    $_SESSION['customers_status']= xtc_db_fetch_array($customers_status_query);
  }else{
    unset($_SESSION['customer_id']);
    xtc_redirect(xtc_href_link(FILENAME_LOGOFF));
  }
}else{
  $customers_status_query = xtc_db_query("
    SELECT * FROM "
.TABLE_CUSTOMERS_STATUS."
    WHERE customers_status_id = '"
.DEFAULT_CUSTOMERS_STATUS_ID_GUEST."'
    AND language_id = '"
. $_SESSION['languages_id'] . "'");
  $_SESSION['customers_status']= xtc_db_fetch_array($customers_status_query);
}

gefällt mir, gute Idee.

Bei aktivierten Kundengruppenrechten lässt sich die Sicherheitslücke übrigens nicht ausnutzen.

Gruß, h-h-h

Offline noRiddle

  • Experte
  • *****
  • Beiträge: 9.736
  • Geschlecht: Männlich
    • Teile Beitrag
    • Webdesign Bonn - Köln
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #29 am: 18. Dezember 2014, 20:49:21 »
Ich habe soeben Beiträge aus dem Thema gelöscht, die quasi eine Anleitung zur Ausnutzung des Fehlers liefern.

Grüße

Torsten

Jau, jau, sorry. :hust: Dachte das wäre ohnhin bekannt und wenn nicht wäre es aus der Code-Diskussion ableitbar gewesen, zumindest für Leute die sowas ausnutzen wollen.

@h-h-h
Mhh, mit dem Nachteil, daß viele Code-Stellen geändert werden müssten, denn die Session-Variable um die es hier ging heißt ja nun $_SESSION['customers_status']['customers_status_id'] und hieße dann $_SESSION['customers_status']['customers_status'].

Gruß,
noRiddle

Offline h-h-h

  • modified Team
  • *****
  • Beiträge: 4.563
    • Teile Beitrag
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #30 am: 18. Dezember 2014, 21:11:07 »
@h-h-h
Mhh, mit dem Nachteil, daß viele Code-Stellen geändert werden müssten, denn die Session-Variable um die es hier ging heißt ja nun $_SESSION['customers_status']['customers_status_id'] und hieße dann $_SESSION['customers_status']['customers_status'].

Naja, so ähnlich wird es in der nächsten Version schon gemacht, ging mir eigentlich nur um die gesparte Abfrage.

Code: PHP  [Auswählen]
  if (isset($_SESSION['customer_id'])) {
    $customers_status_query_1 = xtc_db_query("SELECT customers_status
                                                FROM "
. TABLE_CUSTOMERS . "
                                               WHERE customers_id = '"
. $_SESSION['customer_id'] . "'");

    if (xtc_db_num_rows($customers_status_query_1) == 1) {
      $customers_status_value_1 = xtc_db_fetch_array($customers_status_query_1);

      $customers_status_query = xtc_db_query("SELECT *
                                                FROM "
. TABLE_CUSTOMERS_STATUS . "
                                               WHERE customers_status_id = '"
. $customers_status_value_1['customers_status'] . "'
                                                 AND language_id = '"
. $_SESSION['languages_id'] . "'");

      $_SESSION['customers_status'] = xtc_db_fetch_array($customers_status_query);

      if ($customers_status_value_1['customers_status'] == '0' && !defined('RUN_MODE_ADMIN')) {
        $_SESSION['customers_status']['customers_status_id'] = DEFAULT_CUSTOMERS_STATUS_ID_ADMIN;
        $_SESSION['customers_status']['customers_status'] = $customers_status_value_1['customers_status'];
      } else {
        $_SESSION['customers_status']['customers_status_id'] = $customers_status_value_1['customers_status'];
        $_SESSION['customers_status']['customers_status'] = $customers_status_value_1['customers_status'];
      }
    } else {
      unset($_SESSION['customer_id']);
      xtc_redirect(xtc_href_link(FILENAME_LOGOFF, '', 'SSL'));
    }
  } else {
    $customers_status_query = xtc_db_query("SELECT *
                                              FROM "
. TABLE_CUSTOMERS_STATUS . "
                                             WHERE customers_status_id = '"
. DEFAULT_CUSTOMERS_STATUS_ID_GUEST . "'
                                               AND language_id = '"
. $_SESSION['languages_id'] . "'");

    $_SESSION['customers_status'] = xtc_db_fetch_array($customers_status_query);
    $_SESSION['customers_status']['customers_status_id'] = DEFAULT_CUSTOMERS_STATUS_ID_GUEST;
    $_SESSION['customers_status']['customers_status'] = DEFAULT_CUSTOMERS_STATUS_ID_GUEST;
  }


EDIT:

Somit könnte dies hier die beste Version sein (danke pl1, für die Idee):

Code: PHP  [Auswählen]
     if (isset($_SESSION['customer_id'])) {
        $customers_status_query = xtc_db_query("
         SELECT c.customers_status, cs.*
           FROM "
. TABLE_CUSTOMERS . " c
           JOIN "
. TABLE_CUSTOMERS_STATUS . " cs
             ON cs.customers_status_id = c.customers_status
          WHERE customers_id = "
. $_SESSION['customer_id'] . "
            AND cs.language_id = "
. $_SESSION['languages_id']);
        if (xtc_db_num_rows($customers_status_query)) {
          $_SESSION['customers_status'] = xtc_db_fetch_array($customers_status_query);
          if ($customers_status_query['customers_status'] == '0' && !defined('RUN_MODE_ADMIN')) {
            $_SESSION['customers_status']['customers_status_id'] = DEFAULT_CUSTOMERS_STATUS_ID_ADMIN;
          }
        } else {
          unset($_SESSION['customer_id']);
          xtc_redirect(xtc_href_link(FILENAME_LOGOFF, '', 'SSL'));
        }
      } else {
        $customers_status_query = xtc_db_query("
         SELECT *, customers_status_id as customers_status
           FROM "
. TABLE_CUSTOMERS_STATUS . "
          WHERE customers_status_id = "
. DEFAULT_CUSTOMERS_STATUS_ID_GUEST . "
            AND language_id = "
. $_SESSION['languages_id']);
        $_SESSION['customers_status'] = xtc_db_fetch_array($customers_status_query);
      }
    ?>

Gruß, h-h-h

Offline Archetim

  • Mitglied
  • ***
  • Beiträge: 133
    • Teile Beitrag
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #31 am: 21. Dezember 2014, 06:06:43 »
Danke für die Info auch von uns.

Grüße
Rene

Offline piru

  • Fördermitglied
  • *****
  • Beiträge: 1.263
  • Geschlecht: Weiblich
    • Teile Beitrag
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #32 am: 22. Dezember 2014, 09:12:15 »
:thx:

Offline Buggyboy

  • Fördermitglied
  • *****
  • Beiträge: 916
  • Geschlecht: Männlich
    • Teile Beitrag
    • Spiel-Zeit-Shop
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #33 am: 22. Dezember 2014, 13:10:26 »
Moin!

Auch von mir, vielen Dank für das Bereitstellen des Patches.

Weihnachtliche Grüße
Peter

Offline kaisa

  • Schreiberling
  • ****
  • Beiträge: 360
    • Teile Beitrag
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #34 am: 25. Dezember 2014, 13:00:56 »
Patch hätte ich mir schenken können, da ich Kundengruppen aktiv habe. Nun ja. Drin ist drin.

Ich beschränke mich lieber auf offizielle Patches, denn ich kann den Diskussionen nicht folgen, was denn nun sinnvoll/richtig/funktionierend ist. Sonst baue ich etwas ein und n paar Stunden später wird das revidiert.

Danke und n paar schöne Feiertage.

Offline noRiddle

  • Experte
  • *****
  • Beiträge: 9.736
  • Geschlecht: Männlich
    • Teile Beitrag
    • Webdesign Bonn - Köln
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #35 am: 26. Dezember 2014, 12:34:29 »
Hättest du dir nicht schenken können, denn 1. kann es ja mal sein, daß du irgendwann doch keine Kundengruppen mehr aktiviert hast und bis dahin vergessen hast was hier das Thema ist und 2. kommt anstelle der Sichtbarkeit der Admin-Box eine MySql-Fehlermeldung wenn du Kundengruppen aktiviert hast und jemand die Lücke ausnutzen möchte.

Der Diskussion hier mußt du ja auch nicht folgen, der Patch in Post 1 ist ja offiziell, was also ist das Problem ?

Gruß,
noRiddle

Offline fishnet

  • Fördermitglied
  • *****
  • Beiträge: 4.601
  • Geschlecht: Männlich
    • Teile Beitrag
    • Fishnet Services
Re: Sicherheitspatch für alle Shopversionen (security_fix_2014_12_17.zip)
« Antwort #36 am: 12. Januar 2015, 12:56:09 »
Auch nach vier Wochen ist der Sicherheitspatch im Download nicht enthalten.
Manchmal tun mir die Händler, die sich das selbst herunterladen, echt leid  :crazy:

Offline Jami

  • Neu im Forum
  • *
  • Beiträge: 19
  • Geschlecht: Männlich
    • Teile Beitrag
Hallo,

ich habe mir einige Threads durchgelesen, unter Anderem auch diesen hier.
Den Patch/Fix/wie auch immer man das nennen mag habe ich umgesetzt.

Ich habe allerdings weiterhin folgendes Problem (vermischte Bestellungen): Vermischte Bestellungen [kritisch] was ich mittlerweile auf den hier beschriebenen Fehler zurückführe.

Warum ich hier rein schreibe: Ich habe keine Möglichkeit zu testen ob dieser Fehler behoben wurde, da wir diesen als kritisch ansehen (Kunden erhalten Daten von anderen Kunden) wäre ich sehr dankbar über eine Antwort wie sich eben diese Problematik testen lässt. Wenn nicht öffentlich dann wäre ich auch über eine persönliche Nachricht sehr sehr dankbar.

Ich verzweifle langsam an diesem mysteriösen Fehler.

Offline Morka

  • Fördermitglied
  • *****
  • Beiträge: 75
    • Teile Beitrag
[...]
Ich habe allerdings weiterhin folgendes Problem (vermischte Bestellungen): Vermischte Bestellungen [kritisch] was ich mittlerweile auf den hier beschriebenen Fehler zurückführe.

Warum ich hier rein schreibe: Ich habe keine Möglichkeit zu testen ob dieser Fehler behoben wurde, da wir diesen als kritisch ansehen (Kunden erhalten Daten von anderen Kunden) wäre ich sehr dankbar über eine Antwort wie sich eben diese Problematik testen lässt. [...]

"Wir" heißt ein Team?!
EINE ERSTE Möglichkeit zum Testen könnte sein, dass "Ihr" euch mit euren Email-Konten jeweils ein Test-Kundenkonto erstellt und Testbestellungen macht - oder wenn Du - vermutlich - auch mehrere Email-Konten hast, kannst Du dass natürlich alleine mit mehreren Testkonten testen ...

Viele Grüße

Morka


Teile per facebook Teile per linkedin Teile per twitter

 


             
anything