-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(Mehr) Schutz gegen SQL injection #1474
Comments
@nelarsen vielen Dank für den Hinweis und das Security-Review. Grundsätzlich halte ich es für sinnvoll, weitere Absicherungen gegen SQL Injections zu integrieren. Vor allem aber bei den Stellen |
Danke für den Hinweis zum möglichen Hintergrund. Das ist auf jeden Fall Anlass, die Änderung hinsichtlich Performance zu testen. Ich glaube jedoch nicht, dass es einen Unterschied macht, weder positiv noch negativ. Ich glaube, der Grund war eher, dass es ein bisschen fummelig ist, Abfragen mit Arrays mit prepare() aufzubauen, weil man eine veränderbare Anzahl placeholders benötigt. An einer Stelle habe ich es entsprechend umgebaut und getestet (noch nicht hinsichtlich Performance). Es werden exakt die gleichen SQL-Queries wie davor ausgeführt. Was hält ihr davon? (Ich bin nicht so überzeugt von meiner Änderung) |
Ich würde es lassen. Als ich die Issue geschreiben habe, hatte ich mich noch nicht so intensiv mit der Sache auseinandergesetzt. Danach probierte ich eine "Lösung" aus, aber ich fand dabei das gemeldtete Problem viel weniger dramatisch als ursprünglich angenommen UND meine "Lösung" ist nicht sehr leicht zu lesen/verstehen. Daher würde ich schließen. |
Ein Mitglied hat ein ganz kleines security-Review von CB2 gemacht; insbesondere haben wir uns zusammen angeschaut, ob sorgfältig mit SQL-Anfragen umgegangen wird, sodass SQL injection nahezu ausgeschlossen ist. Das ist fast überall der Fall, weil fast überall $wpdb->prepare() verwendet wird: Sehr gut! Wir haben jedoch vier Stellen gefunden, wo das nicht der Fall ist. In diesen Fällen ist es schwieriger zu prüfen, ob es eine potentielle Sicherheitslücke gibt. Ich schlage vor, diese vier Stellen ebenfalls in prepare() zu ändern, um SQL injection auszuschließen. Die vier Stellen sind in:
CB1::getCB1Taxonomies()
Restriction::queryPosts()
Timeframe::getPostIdsByType()
Timeframe::getPostsByBaseParams()
Ich mache gern ein PR wenn gewünscht.
The text was updated successfully, but these errors were encountered: