View Issue Details

IDProjectCategoryView StatusLast Update
0000147Adventure PHP FrameworkSicherheit // Securitypublic2017-03-18 18:23
ReporterScreezeAssigned ToChristianAchatz 
PrioritynormalSeverityfeatureReproducibilityN/A
Status resolvedResolutionfixed 
Product Version2.0 
Target Version3.3Fixed in Version3.3 
Summary0000147: Automatischer XSS Schutz in Placeholdern
DescriptionIch hatte eben eine Idee, die Entwicklern helfen soll Cross-Site-Scripting Lücken von vornherein zu vermeiden.
Klar sollte der Entwickler darauf achten, immer alle Informationen die vom User direkt oder indirekt kommen zu escapen, um XSS zu vermeiden.

Wir könnten dem APF aber ein grundlegendes Sicherheitsfeature mitgeben, dass die Entwickler unterstützt und eine zusätzlich Schutzbarriere bietet, falls er es mal vergisst. Das würde das APF bezüglich Sicherheit einen Schritt nach vorne bringen, und könnte zusätzlichen Anreiz bieten es zu verwenden.

Und zwar ist die Idee, allen *:placeholder Tags ein Flag zu geben, welches angibt, ob dieser Platzhalter nur reinen Text beinhalten darf, oder auch HTML rendern darf.

z.B.: <core:placeholder name="Test" htmlallowed="true" />

Wird html erlaubt, wird die Ausgabe einfach 1:1 gerendert. Wird html nicht erlaubt, wird der ersetzende Text durch htmlentities geschickt. Eventuell könnte man noch dafür sorgen, dass Zeilenumbrüche in form von br-Tags erlaubt werden über ein 2. Flag.

Nun gäbe es 2 Ansätze, das umzusetzen:

1. Defaultmäßig html verbieten, nur per Attribut erlauben:
Vorteil: Standartmäßige Sicherheit ohne Aufwand. Neue Benutzer profitieren automatisch davon.
Nachteil: Migration alter Anwendungen aufwändig, da Platzhalter entsprechend mit Attributen markiert werden müssen.

2. Normalerweise html erlauben, per attribut verbieten:
Vorteil: Migration alter Anwendungen kein Problem, da alles belassen werden kann wie es ist.
Nachteil: Die wenigsten werden von dem Attribut gebrauch machen, weil es vermutlich zu unbekannt wäre. Wenig Sicherheitsgewinn.

Ich schlage daher einen Kompromiss vor:
Wir könnten das Feature standartmäßig aktivieren, sprich: HTML verbieten.
Zusätzlich bieten wir aber einen Registry-Einrag an, über den global geregelt werden kann, dass standartmäßig Html erlaubt ist, man über htmlallowed="false" aber einzelne Tags definieren kann, die kein HTML erlauben.

Dann könnte man für alte Anwendungen, bei denen man sich den Migrationsaufwand sparen will, einfach folgendes in die Bootstrap Datei packen:
Registry::register(PlaceHolderTag::HTML_ALLOWED_DEFAULT_KEY, false);

Man müsste lediglich alle APF-Module und Anwendungen auf das Feature migrieren, aber das sollte machbar sein.
TagsNo tags attached.
Codereferenz: ([Datei]:[Zeile])
Namespacetools

Activities

jwlighting

2014-02-19 17:40

administrator   ~0000225

Last edited: 2014-02-19 17:41

View 2 revisions

Hallo Ralf,

ich finde die Idee nicht schlecht. Ich würde auch deinen Kompromiss anwenden, habe jedoch eine weitere Idee:

Wir erweitern Document um eine Eigenschaft, die das Verhalten definiert und an seine Kinder weiter gibt, ansprechbar über ein Attribut oder einen DocCon.
So lässt sich das Verhalten auch für einzelne Templates anpassen. Auch die Migration würde dadurch leichter.

LG :)

PS: Haben wir eigentlich schon ein ähnliches Feature für SQL-Injection?
Jan

dingsda

2014-02-22 19:51

developer   ~0000226

ich find die idee auch sehr gut.
ist man nicht aber flexibler, wenn man den flag in die funktion setPlaceHolder einbaut?

@ sql-injection: soweit ich weiß wird alles escaped was zur db geht. überprüft hab ich es allerdings nicht, weil ich noch nicht ganz durchschaut habe wo welche querys zusammengebaut werden.

Screeze

2014-02-22 20:13

developer   ~0000227

SQLi wird zumindest im GORM verhindert. Mit der normalen DB-Komponente geht das logischerweise nicht, dafür gibts ja die Prepared statements.

Der Flag in Setplaceholder ist ja trotzdem geplant.
Finde jwlightings Idee gut :)

ChristianAchatz

2014-05-14 00:23

administrator   ~0000338

Postponed to get 2.1 released soon (as aligned with Ralf).

ChristianAchatz

2015-03-07 13:40

administrator   ~0000547

Moved to 3.1 to be able to release 3.0 soon.

ChristianAchatz

2015-09-05 17:04

administrator   ~0000591

Moved to 3.2 to shape scope for 3.1.

ChristianAchatz

2016-06-27 14:38

administrator   ~0000718

Implementation idea: provide 2 implementations for PlaceHolderTag, one as-is and one with automatic escaping. Which place holder tag to use can easily be determined by template expression registered.

ChristianAchatz

2017-03-15 20:14

administrator   ~0000765

* Added documentation of solution approach under http://wiki.adventure-php-framework.org/XSS_Schutz_f%C3%BCr_Platzhalter.
* Added clearTemplateExpressions() method to DomNode and Document to allow easy customization and replacement
 of template expressions w/o custom bootstrapping.
* Updated official documentation on new method clearTemplateExpressions().

ChristianAchatz

2017-03-18 18:23

administrator   ~0000769

Documentation translated to English: http://wiki.adventure-php-framework.org/XSS_Schutz_f%C3%BCr_Platzhalter/en

Issue History

Date Modified Username Field Change
2014-02-19 09:15 Screeze New Issue
2014-02-19 09:16 Screeze Description Updated View Revisions
2014-02-19 17:40 jwlighting Note Added: 0000225
2014-02-19 17:41 jwlighting Note Edited: 0000225 View Revisions
2014-02-22 19:51 dingsda Note Added: 0000226
2014-02-22 20:13 Screeze Note Added: 0000227
2014-02-24 17:32 jwlighting Category Neues Feature // New Feature => Sicherheit // Security
2014-05-14 00:23 ChristianAchatz Note Added: 0000338
2014-05-14 00:23 ChristianAchatz Target Version 2.1 => 3.0
2015-03-07 13:40 ChristianAchatz Note Added: 0000547
2015-03-07 13:40 ChristianAchatz Target Version 3.0 => 3.1
2015-09-05 17:04 ChristianAchatz Note Added: 0000591
2015-09-05 17:04 ChristianAchatz Target Version 3.1 => 3.2
2016-03-23 14:01 ChristianAchatz Target Version 3.2 => 3.3
2016-06-27 14:38 ChristianAchatz Note Added: 0000718
2017-03-15 20:14 ChristianAchatz Note Added: 0000765
2017-03-16 12:35 ChristianAchatz Assigned To => ChristianAchatz
2017-03-16 12:35 ChristianAchatz Status new => assigned
2017-03-17 18:18 ChristianAchatz Status assigned => resolved
2017-03-17 18:18 ChristianAchatz Fixed in Version => 3.3
2017-03-17 18:18 ChristianAchatz Resolution open => fixed
2017-03-18 18:23 ChristianAchatz Note Added: 0000769