Swift-Gallery

[dbs]

Anscheinend ist das Teil noch nie auf Sicherheit getestet worden

Da solltest du dich an unseren Sicherheitsbeauftra gten evaki wenden. Aber der macht eigentlich was er will. 

[evaki]

Hab dat Dingens anscheinend gefunden.
view.php
   

Code: [Select]

<?php
if ($number_of_galleries>1) {
$i=0;
foreach($db_array as $gall) {
echo("<a href=\"?gallery_id={$gall['gallery_id']}\">{$gall['name']}</a>");
$i++;
if ($i<$number_of_galleries) echo(" - ");
}
}
?>
Das (echo("<a href=\"?gallery_id   ) muß nach erstem Eindruck wohl mit "htmlspecialchars" gesichert werden.
Hat jemand färtisch, bevor ich färtisch?


Jo >>>>>>> Ich habe anscheinend färtisch

MfG. Evaki

[jacobi22]

hab eigentlich nicht mehr vor, mich da rein zu hängen und die Tips um natsort() blieben ja auch ungelesen

übergeben wird per Get die Gallery-ID und der Start-wert - beides hat Null Absicherung aktuell (hab gestern das letzte Mal in eure scripte geschaut)

aus dem Code der Vers.2.0.0.dev1, die grad im Test bei ein paar Leuten ist. Der Code müßte dann auf die Varablenamen der alten Swiftversion angepasst werden

if (isset($_GET['gallery_id']) AND is_numeric($_GET['gallery_id'])) {
        $iGalleryId = (int)$_GET['gallery_id'];
    } else {
        $iGalleryId = $aTwigdata['settings']['default_gallery'];
    }

    
    if (isset($_GET['start']) AND is_numeric($_GET['start'])) {
        $iStart = (int)$_GET['start'];
    } else {
        $iStart = 0;
    }

[evaki]

natsort() blieben ja auch ungelesen

Nein, da irrst Du. Lokal ist das hier drin, aber mehrfach nicht übernommen worden.
Soweit ich erinnere, gab es eine Fehlermeldung wg. Klammer, wobei dann versehentlich nicht nur die Klammer weg war. Passierte komischerweise heute schon wieder.  Kommt am Ende aber wieder rein !

So, nun mach ich erstmal Pause.

[dbs]

Deine Lösung bezog sich aber nicht auf mein Problem, oder?
Da wäre die modify.php der richtige Ort, denn es geht ja um das ausgewählte Verzeichnis. Dieses ist nach Speichern nicht mehr ausgewählt, weshalb nochmaliges Speichern wegen zB Änderung des Titels, ein leeres Verzeichnis speichert. Hab ich wohl schlecht ausgedrückt und war vlt. auch vorher schon so.

[evaki]

Dieses ist nach Speichern nicht mehr ausgewählt, weshalb nochmaliges Speichern wegen zB Änderung des Titels, ein leeres Verzeichnis speichert.

Genau das war mMn schon in der Vorlage so.
Wenn man aber das zum Galerienamen gehörende Verzeichnis anwählt, und dann den Titel ändert, scheint es zu funktionieren, wenn ich recht erinnere. Ist natürlich doof, wenn man dafür erst ins FE gucken muß. Das war aber genau das, was mir schon früh auffiel.

Da ich nun den Sicherheitsschei... an der Backe habe, geht das erstmal vor.
"htmlspecialchars" war ein Schnellschuß, da der mir anscheinend wg. der Konvertierung die Syntax zerhaut. Ich weiß genau, warum ich mir das php-Zeugs im "normalen" Leben nicht antue.
Mit'm Hammer und Nägel kann man ein ganzes Haus zusammenbauen. (ich weiß, mit php auch  )

[evaki]

@jacobi22
Dein Beispiel kam gerade recht, als ich merkte, daß ich direkt bei der gallery_id ansetzen muß,
  - (Schlußfolgerung aus dem "ich probier das mal mit Filtern"-Schnellschuß) -
Schön, daß ich mich nun mit den Unterschieden von $_REQUEST,  $_GET und $_POST beschäftigen darf, statt Schoppen zu gehen 
Das Leben könnte soooo schön sein.

Da kommt noch was: Sehe soeben, daß -ich habs über request gemacht, - die XSS + injection Meldungen ausblieben. Das wichtigste scheint an dieser Stelle wohl die Verknüpfung/Bedingung (AND is_numeric) zu sein (da jaulte der Scanner) Falls die Galerie jetzt noch immer funktioniert,  könnte es das gewesen sein. Nochmal 'nen kompletten Durchlauf machen.

MfG. Evaki

[jacobi22]

nur mal theoretisch  - $REQUEST ist beides, GET und POST. Bekannt ist die Funktion der beiden Variablen, es wird die Galerie-ID und der Startwert übergeben, jeweils eine Zahl. Wir wissen weiterhin, das dies nur per GET vom Original-Script kommen kann, also macht man die POST-Tür schon mal zu, indem man kein $REQUEST nimmt.
Ohne die Festlegung auf numerisch kann jeder Wert gesendet werden, also auch eine SQL-Injection, ein Javascript usw. Diese würde aber nicht gehen ohne Buchstaben und/oder Sonderzeichen. Auf Anhieb kenn ich da keine, die so funktionieren würde.
Natürlich ließe sich auch der obige Code noch weiter optimieren, in dem man z.b. die in Frage kommenden Gallery-ID und die Range der möglichen Startwerte ausliest, dann dürfte als Beispiel gallery_id nur die Werte 1,2,4 und 5 haben (weil ich nur diese Galerien angelegt habe), nicht etwa 3 und auch nicht 6 und Start dann nur 1,11,21, 31 usw, nicht aber 32 oder 38.
Und kommt ein Wert falsch an, weil bewußt versucht, etwas anderes als das Erlaubte zu versuchen,  oder fehlerhaft übermittelt, am Schluß dann die Absicherung auf die Default_Gallery-ID und den Startwert 0 bzw 1

Aber wie du oben schon sagtest, ist seit 2006 so und vom Grundprinzip arbeiten alle Module mit Seiten-Pagination ähnlich - heißt aber nicht, das es so bleiben muß 

[evaki]

heißt aber nicht, das es so bleiben muß

Weshalb Du ja was neues machst, und ich "nur" versuche - man vergesse nicht, daß ich Laie bin - das 2006er-Modul an die aktuelle Umgebung anzupassen. Wobei dann immer wieder so'n büsschen Lerneffekt dabei ist. Programmieren überlaß ich sinnvollerweise denen die's können.
(Das hatte ich mal gaaaanz früher, auf 'ner komplett anderen Schiene, weshalb das Mitdenken an einigen Stellen noch fruchtbar ist. Das isses dann aber auch schon.)
MfG. Evaki

[jacobi22]

War das nicht gut geschrieben zum Verstehen?   

War kein Vorwurf an Niemanden und auch an keinen speziellen Empfänger gerichtet. Solang das Zeugs OpenSource ist und es dementsprechend auch viele Altlasten gibt, wird man damit leben müssen, das Leute auch ihr eigenes Kram machen, auch dafür bietet WB ja ideale Voraussetzungen. Vielleicht hätte man es in den Anfangsjahren schon strenger limitieren müssen, speziell diese Sache war ja damals auch schon relevant

[evaki]

War das nicht gut geschrieben zum Verstehen?

Das ist alles so in Ordnung. 
Bringt mich ja auch stückchenweise weiter, muß ich nicht jedesmal das Netz neu durchsuchen, bzw. weiß dann wo ich nachschauen muß, wenn ich weiß wo's langgeht, langgehen soll.
Besser als zig fruchtlose Verirrungen. So haben wir alles was davon.

So wie es aussieht, funktioniert der Sicherheitsfix und die Galerie funktioniert wie gehabt.

Da könnte ich ja mal auf weitere Dinge schauen   
1.) Es gibt 2 Notices, wenn man die Einstellungen vor dem Erstellen einer Galerie tätigt.
2.) Die fehlende Pfadangabe, die mehrmals erwähnt wurde.

Ich glaub, erstmal den Sicherheitsfix raushauen, dann 'ne Pause zum Nachdenken einlegen.
Bin ja schon froh, daß ich soweit gekommen bin.
MfG. Evaki

[jacobi22]

noch zur Info, ich weiß nicht, ob ich es schon geschrieben hatte - bin mit der neuen Version jetzt bei 2.0.0dev2 - da sollte dann genug Platz dazwischen sein.
Kannst ja auch noch weiter unterteilen, z.b. 0.7.145.299 oder 0.7dev1234567889

P.S.: der Heinz (bbs2) hat diese Version zum Test, ging natürlich in die Hose. War bei Strato. Weiß nicht, ob da ein Cache System oder PHP Booster aktiv sind. Lt error.log suchte er Dateien, die lt FTP aber vorhanden sind. Der Rest waren Folgefehler. Ich hab das jetzt bei 5 Anbietern laufen (u.a. strato, AllInkl, One.com) und funktionierte alles mit Ausgangsbasis 0.6 oder 0.7

[evaki]

Betrifft: XSS und sql injection
Unbedingt installieren/updaten
swift_gallery_v0.7. 2-dev-secufix01.zip
Und wenn möglich Rückmeldung, möchte keinen Schrott hinterlassen.


@jacobi22

Kannst ja auch noch weiter unterteilen

Genauso dachte ich mir das auch. Denke, daß sich das irgendwann auch von selbst erledigt, allein wg. unflexiblem Design. Wie wir wissen, halten sich Templates manchmal ganz schön lange, eben auch die von 2006  , was aber manchenorts noch durchaus seine Berechtigung hat. Hatte ich mal bei 'ner Kirchensite. Da ist's halt auch gut, daß nur auf der technischen Seite "upgedatet" wird.

Gut ist ja, daß Du so umsichtig bist, und das auf mehreren recht unterschiedlichen Plattformen laufen läßt.
MfG. Evaki

[jacobi22]

Denke, daß sich das irgendwann auch von selbst erledigt, allein wg. unflexiblem Design. Wie wir wissen, halten sich Templates manchmal ganz schön lange, eben auch die von 2006  , was aber manchenorts noch durchaus seine Berechtigung hat. Hatte ich mal bei 'ner Kirchensite. Da ist's halt auch gut, daß nur auf der technischen Seite "upgedatet" wird.

die neue Modulestruktur gibt es ja her, das dann das eigene Design nicht mehr überschrieben wird und man könnte sich auch für jede Page ein eigenes Template im Modul bauen, das dann auch im Moduleupgrade sicher ist und nicht überschrieben wird. Der Haken ist halt, das man nicht weiß, was sich der Anwender vielleicht selbst angepasst hat - aber irgendwann muß man den Schritt ins Neue mal gehen. Ist beim UTF8 genauso. Einmal umgestellt, ist erstmal Ruhe für eine lange Zeit

[evaki]

war nix. dumm gelaufen

[evaki]

So, nochmal. Da war noch'n Fehler drin.

Anbei modify.zip für die Anzeige des Pfades unter "Galerie wählen".

Der Punkt "Pfad zu den Bildern" ist tatsächlich nur zum Setzen einer Galerie gedacht/vorgesehen.
Doof, aber ist so.

Bitte testen und mitteilen, ob das als nützlich angesehen wird. Dann wird's im nächsten Paket drin sein. 

MfG. Evaki

Evaki:
Da könnte ich ja mal auf weitere Dinge schauen   
1.) Es gibt 2 Notices, wenn man die Einstellungen vor dem Erstellen einer Galerie tätigt.
2.) Die fehlende Pfadangabe, die mehrmals erwähnt wurde.

Punkt 2 wäre damit erledigt
Punkt 1 tritt jederzeit unter "Einstellungen" auf
Hoffe, daß ich dafür noch Zeit (hier, gibt ja auch noch anderes...) bekomme.

[dbs]

Mach mal deine letzten Änderungen wegen dem Pfad rückgängig und ändere nur in Zeile 121 zweimal <?  zu  <?php
Dann wird der Pfad bzw. das Verzeichnis angezeigt.

[dbs]

Ich häng dir mal 2 Dateien ran. settings.php & backend.css

Der Validator sagte da sind ungeschlossene Divs und ein streunendes end form.
Bereinigt, obwohl ich nicht verstehe wie da die end divs fehlen können.
Außerdem Tannebaumansicht, also die Werte nicht am rechten Rand, sondern links bei den labels.

[jacobi22]

da fehlt der settings ein print_footer(), deshalb macht er die in der admin/pages/modify.php geöffneten Div's nicht zu
sieht man besonders im Quelltext, der nach der settings abbricht, aber auch im Backend am fehlenden Footer

am Schluß der settings.php diese Zeilen

Code: [Select]

<?php
if ($print_info_banner) { ?>
<?php
}

// Print admin footer
$admin->print_footer();
ein Tipfehler war noch in Zeile 96 - Leerzeichen vor styles

Code: [Select]

<td class="setting_name"style="width: 30%;">
das schließende </form> aus Zeile 124 dann hinter das Ende des </table> in Zeile 131, aber das war ja schon in der zuletzt geposteten Datei.
Die drei eingefügten div gehören da aber nicht rein

[evaki]

Guten Morgen.
Hat also doch noch'n Rattenschwanz.
Alles Dinge, die nicht überprüft wurden, oder auch zum aktuellen Zeitpunkt nicht überprüft werden konnten.

Vielleicht braucht es bei alten oder generell bei Modulen eine andere Strategie, wenn Updates anstehen.
Nämlich zuerst auf alles mögliche wie html und Logik prüfen, bevor man sich den eigentlichen - gemeldeten - Fehlern zuwendet.
Das würde dem entsprechen, was ich sonst bei html- und Serverchecks auch mache.

Apropos Logik, die kann ich selbst nur an einzelnen und wenigen Stellen nachvollziehen, weshalb dies das Feld für die Programmierer sein/bleiben sollte. Denn wie man hier gut sehen konnte, führt dies zu Fehlinterpretatione n bzw. falschen Schlüssen, wenn man ohne solide Kenntnisse herangeht. Hatte hier nicht die Notwendigkeit gesehen, die Scripte einem Codecheck (Tool) zu unterziehen, was ich schon allein wg. drohender Überforderung äußerst selten mache. Tja, und scheinbar war ja alles auch in Ordnung.

Kurz gesagt, ohne Zutun aus der Programmierecke würde ich jetzt feststecken.
Aus unserem Anwenderkreis konnte auch nicht mehr kommen, wie man jetzt feststellen kann; sind ja auch alles Laien, wenn auch mit  - Danke sei wem auch immer - viel Phantasie.
MfG. Evaki

[evaki]

Zeitlimit hatte zugeschlagen

....auch alles Laien, wenn auch mit  - Dank sei wem auch immer - viel Phantasie.
Schön, daß es bei diesem Teil nun doch zu einem Abschluß kommen kann.
MfG. Evaki

[evaki]

Mach mal deine letzten Änderungen wegen dem Pfad rückgängig und ändere nur in Zeile 121 zweimal <?  zu  <?php
Dann wird der Pfad bzw. das Verzeichnis angezeigt.

Hat bei mir (noch) nix gebracht. Werde es aber im Laufe dea Tages nochmal komplett neu installieren.

Außerdem habe ich den Eindruck, daß da was (settings) durcheinander gerät, wie z.B.
v01.) if ($print_info_banner) {
v02.) class="w3-btn w3-blue-wb"

Hab' nun $admin->print_footer(); aus 01 übernommen, ohne  if ($print_info_banner) {

MfG. Evaki

[evaki]

Es gibt aber noch ein dringliches Anliegen.
Hat man das (egal ob altes oder neues ) Modul und noch keine Gallerie installiert ,  werden bei Eingabe von z.B. http://localhost/4-seite.php?gallery_id=1 die Dateien in root (localhost/) gelistet. Sowas meldet kein Scanner 

Bevor ich mir nun wieder den Wolf suche, und möglicherweise nur Rotkäpchen finde, hätt' ich für 'ne Lösung ein offenes Ohr.

MfG. Evaki
ps. falls das mit der "Pfadgeschichte" nicht aufgeht, lege ich zumindest 'ne alternative modify bei.

[dbs]

Es muss durcheinanderkommen, wenn jeder eine settings.php postet.
Du könntest es aber auseinanderfummeln bzw. die jeweiligen Änderungen erkennen.

ZB bei meiner settings die letzten drei div entfernen und dafür jacobis admin print footer nehmen.
Alles andere hatte ich schon gemacht. Mit der backend.css sollte es dann (nur) dort gut aussehen.

v02.) class="w3-btn w3-blue-wb"

Die Klassen machen blaue Buttons. Kannst du beide weglassen.

[evaki]

drei div entfernen und dafür jacobis admin print footer nehmen.

Habe soeben mal reingeschaut v.0.7.xxx, ist so auch drin 

Am "alten Aussehen" soll bei der "alten" Version auch nix mehr geändert werden.
Jacobi überarbeitet auch das ja mit seiner neuen.