View Issue Details

IDProjectCategoryView StatusLast Update
0000180Adventure PHP FrameworkSicherheit // Securitypublic2015-10-12 12:20
ReporterdingsdaAssigned ToChristianAchatz 
PriorityurgentSeveritymajorReproducibilityhave not tried
Status closedResolutionfixed 
Product Version2.0 
Target Version2.1Fixed in Version2.1 
Summary0000180: DBHandler sollten die API funktionen zum ändern des charsets nutzen um sql-injektion zu vermeiden
DescriptionAktuell wird das Charset der DB-Verbindung mit "set names ..." geändert. Leider hat das keine Auswirkung auf die Funktionen zum Escapen der Strings und ermöglicht so in manchen fällen noch SQL-Injection. Korrekt sollte daher für den MySQLx- und MySQLi-Handler das Charset mittels der Funktion set_charset geändert werden und beim PDOHandler in den DSN eingebaut werden.
siehe dazu:
http://www.php.net/manual/en/mysqlinfo.concepts.charset.php
TagsNo tags attached.
Codereferenz: ([Datei]:[Zeile])
Namespacecore

Activities

ChristianAchatz

2014-03-24 23:14

administrator   ~0000276

Kannst du eine Einschätzung geben, wann das zu SQL-Injections führen kann? Das Charset sollte doch - zumindest für die relevanten Steuerzeichen - dafür keine (zu große) Rolle spielen...

dingsda

2014-03-25 08:25

developer   ~0000277

Last edited: 2014-03-25 08:28

View 3 revisions

leider kann ich das nicht wirklich. ich bin auch nur zufällig über die seite im php-manual gestolpert und hab nicht so das größte verständnis für das ganze encoding-zeugs. hab versucht was dazu bei google zu finden um das verhalten zu reproduzieren. aber so richtig ist mir das nicht gelungen. es scheint von mehreren faktoren abhängig zu sein:
- default-charset von mysql und encoding in der anwendung (z.b. GBK vs UTF8)
- version von mysql (vor mysql 5.0.77 gabs z.b. nen bug wegen GBK, ob da noch andere bugs später waren weiß ich nicht)

also wahrscheinlich doch eher ein spezieller fall. trotzdem sollte man das wohl in der nächsten version aufnehmen, wenn die classen mysqli und pdo das schon anbieten und es die empfohlene funktion dafür ist.
bei PDO muss man aber zusätzlich noch set names nutzen, da dass charset in der dsn ignoriert wird vor php 5.3.6

ChristianAchatz

2014-03-26 22:47

administrator   ~0000278

Das können wir gerne tun. Magst du mal einen Vorschlag/ein Proposal coden?

dingsda

2014-03-28 16:55

developer   ~0000284

ich hab die veränderten dateien mal hochgeladen.

große änderungen sind es nicht:
PDO: zeilen 382-384
mysqli: zeilen 75-82
mysqlx: zeilen 79-85

hab zusätzlich noch die Methode initCharsetAndCollation des abstractDatabaseHandler bei der gelegenheit geändert. jetzt wird in einem einzigen statement das charset und die collation geändert.

ChristianAchatz

2014-03-28 21:19

administrator   ~0000286

Danke dir! Ich integriere das in den nächsten Tagen in den 2.1 Branch.

ChristianAchatz

2014-05-06 11:26

administrator   ~0000324

Hallo dingsda,

ich eingebaut. Push folgt. Die Nutzung der mysql_set_charset() habe ich korrigiert, dort waren die Argumente vertauscht:

mysql_set_charset($this->dbConn, $this->dbCharset)

vs.

mysql_set_charset($this->dbCharset, $this->dbConn)

Siehe http://de2.php.net/mysql_set_charset.

Issue History

Date Modified Username Field Change
2014-03-23 22:16 dingsda New Issue
2014-03-24 23:14 ChristianAchatz Note Added: 0000276
2014-03-25 08:25 dingsda Note Added: 0000277
2014-03-25 08:25 dingsda Note Edited: 0000277 View Revisions
2014-03-25 08:28 dingsda Note Edited: 0000277 View Revisions
2014-03-26 22:47 ChristianAchatz Note Added: 0000278
2014-03-28 16:55 dingsda Note Added: 0000284
2014-03-28 21:19 ChristianAchatz Note Added: 0000286
2014-05-06 11:13 ChristianAchatz Assigned To => ChristianAchatz
2014-05-06 11:13 ChristianAchatz Status new => assigned
2014-05-06 11:13 ChristianAchatz Target Version => 2.1
2014-05-06 11:26 ChristianAchatz Note Added: 0000324
2014-05-06 11:27 ChristianAchatz Status assigned => resolved
2014-05-06 11:27 ChristianAchatz Fixed in Version => 2.1
2014-05-06 11:27 ChristianAchatz Resolution open => fixed
2015-10-12 12:20 ChristianAchatz Status resolved => closed