Zweimal `square()` - Teil 1

Manchmal beginnt Code-Archäologie nicht mit einem spektakulären Fund. Manchmal beginnt sie mit einem SQL-Injection-Fix – und einer Funktion, die eine Zahl quadriert.

Am 12. August 2026 wurde im LimeSurvey-Repository Pull Request #5225 eröffnet. Der Titel: “Fixed issue #20648: [security] SQL injection in statistics”.

Das Problem war ernst. In der Statistikfunktion konnten manipulierte Filterparameter dazu führen, dass vom Request stammende Feldnamen ungeprüft in SQL-Abfragen gelangten. Der Fix führte deshalb eine Allowlist ein: Nur Spalten, die tatsächlich im Schema der jeweiligen Response-Tabelle existieren, dürfen noch als Feldnamen verwendet werden¹.

Der Pull Request wurde schnell behandelt. Review, Nachbesserungen, QA – einen Tag später war der Fix in master.

So weit, so gut.

Ein KI-gestütztes Review-Tool kommentierte die Änderung übrigens mit einer launigen, gereimten Zusammenfassung² – ein kleines Kuriosum am Rande, das die Ernsthaftigkeit des Fixes nicht schmälert.

Doch beim Blick auf die Änderungen fällt etwas anderes auf.

Denn der Fix wurde nicht an einer Stelle vorgenommen.

Er wurde zweimal implementiert.

Zwei Statistics-Helper

Der Pull Request verändert zwei Dateien:

application/helpers/admin/statistics_helper.php
application/helpers/userstatistics_helper.php

Moment. Zwei Statistics-Helper?

Der erste umfasst heute mehrere Tausend Zeilen. Der zweite ebenfalls. Und beide enthalten eine Funktion namens buildSelects().

Diese Funktion ist für den Security-Fix besonders interessant. Sie verarbeitet unter anderem Filterinformationen aus dem Request, interpretiert LimeSurvey-Feldnamen und Fragetypen und erzeugt daraus Bedingungen für Datenbankabfragen.

Genau dort musste #5225 eingreifen. In beiden Dateien.

Der Fix ermittelt zunächst die tatsächlich existierenden Spalten der dynamischen Response-Tabelle und baut daraus eine Allowlist. Ein aus dem Request abgeleiteter Feldname darf nur weiterverwendet werden, wenn er in dieser Liste vorkommt.

Das ist als unmittelbarer Security-Fix vernünftig. Eine offene SQL Injection ist nicht der richtige Zeitpunkt, um nebenbei ein mehrere tausend Zeilen großes Statistics-Subsystem neu zu entwerfen. Der Fix sollte klein, nachvollziehbar und schnell auslieferbar bleiben.

Aber eine Frage bleibt: Warum existiert die sicherheitskritische Filterlogik überhaupt zweimal?

Eine unscheinbare Funktion

Wer die beiden Dateien nebeneinanderlegt, findet nicht nur zwei Varianten von buildSelects().

Man findet zum Beispiel auch das hier:

function square($number)
{
    if ($number == 0) {
        $squarenumber = 0;
    } else {
        $squarenumber = $number * $number;
    }

    return $squarenumber;
}

Eine Funktion, die eine Zahl quadriert. Sie existiert in beiden Statistics-Helpern.

Das ist für sich genommen weder ein Bug noch ein Security-Problem. Die Funktion ist trivial. Gerade deshalb ist sie interessant: Warum braucht eine Anwendung eine Quadratfunktion für Administratoren und eine zweite Quadratfunktion für Benutzer?

Natürlich braucht sie das nicht. square() kennt weder Benutzer noch Administratoren. Sie kennt nicht einmal LimeSurvey. Sie multipliziert eine Zahl mit sich selbst.

Wenn eine derart kontextfreie Funktion in zwei fachlich ähnlich benannten Dateien separat existiert, ist sie ein kleines Fossil. Sie deutet darauf hin, dass hier irgendwann nicht einzelne Verantwortlichkeiten getrennt wurden, sondern größere Mengen bestehenden Codes einen zweiten Lebensweg begonnen haben.

Also lohnt sich der Blick in die Git-Historie.

Rückwärts

Geht man einige Jahre zurück, findet man Commit-Messages, die bemerkenswert offen beschreiben, was mit den beiden Statistics-Implementierungen passiert ist.

Am 24. Mai 2018 findet sich Commit a009d48:

Dev: Apply statistics fix to user statistics

Das klingt bereits danach, dass eine Änderung aus einem Statistics-Pfad in den anderen übertragen werden musste.

Noch deutlicher wird es am 28. Juli 2017, Commit 8b9caa6:

Dev: Yet third identical fix to user statistics

“Yet third identical fix”. Spätestens hier verdichtet sich der Verdacht: Die Repository-Historie dokumentiert selbst, dass identische Korrekturen in der User-Statistik nachgezogen werden mussten.

Aber woher kam diese zweite Implementierung überhaupt? Dafür müssen wir noch ein Jahr weiter zurück.

7. März 2016

Am 7. März 2016, 14:49 Uhr, landet Commit 90ca87f im Repository:

Fixed issue #10688: No charts in public statistic

Der Anlass klingt unspektakulär. In der öffentlichen Statistik funktionieren die Charts nicht korrekt.

Der Commit ändert den Controller für die User-Statistik. Vorher wird der bestehende Admin-Statistics-Helper verwendet:

Yii::app()->loadHelper("admin/statistics");
$helper = new statistics_helper();

Danach verwendet der Controller einen neuen Helper:

Yii::app()->loadHelper("userstatistics");
$helper = new userstatistics_helper();

Und mit demselben Commit erscheint eine neue Datei: application/helpers/userstatistics_helper.php, 3.342 Zeilen auf einmal.

Das ist der Moment, an dem die heutige Struktur entsteht. Vergleicht man den neu eingeführten User-Helper mit dem damals bereits vorhandenen Admin-Helper, findet man dieselben Funktionen in derselben Struktur. createChart(). buildSelects(). Und natürlich: square().

Der Public-Statistics-Code wurde nicht lediglich über eine kleine Präsentationsschicht vom Admin-Kontext getrennt. Ein großer Teil des bestehenden Statistics-Subsystems bekam eine zweite Variante. Aus einer Implementierung wurden zwei.

War das 2016 ein Fehler?

Aus dem Jahr 2026 lässt sich leicht auf alten Code zeigen und erklären, wie man ihn heute anders strukturieren würde. Das wäre allerdings eine ziemlich billige Form von Code-Review.

Wir kennen die Situation von 2016 nicht. Wir wissen nicht, welche zeitlichen Zwänge bestanden. Wir wissen nicht, wie gut die Statistics-Funktionalität damals durch Tests abgesichert war. Wir wissen nicht, wie riskant eine gemeinsame Abstraktion gewesen wäre. Und wir wissen nicht, welche anderen Anforderungen gleichzeitig auf dem Tisch lagen.

Wir wissen nur: Es gab ein konkretes Problem mit Public Statistics, und die gewählte Lösung hat dieses Problem offenbar gelöst.

Eine pragmatische Entscheidung kann in ihrem damaligen Kontext vollkommen nachvollziehbar sein. Interessanter ist eine andere Frage: Was hat diese Entscheidung anschließend gekostet? Und darauf gibt uns Git eine erstaunlich gute Antwort.

Technical Debt zahlt Zinsen

Mit der Abspaltung 2016 entstand nicht einfach nur duplizierter PHP-Code. Es wurde Wissen dupliziert.

buildSelects() weiß etwas darüber, wie Request-Daten interpretiert werden. Es kennt Survey-Felder, Response-Spalten, Fragetypen und Filterbedingungen. Und es weiß letztlich, wie aus diesen Informationen Datenbankbedingungen entstehen.

Wer eine solche Funktion kopiert, kopiert deshalb mehr als Text:

Code
  ↓
Business Rules
  ↓
Datenmodellwissen
  ↓
Security-Annahmen
  ↓
Trust Boundaries

Ab diesem Moment müssen Erkenntnisse über dieses Wissen an zwei Stellen gepflegt werden. Ein Bugfix wird möglicherweise zweimal nötig. Eine Anpassung an neue PHP-Versionen betrifft möglicherweise beide Varianten. Ändert sich das Response-Schema, müssen beide Implementierungen korrekt damit umgehen. Und eine Security-Erkenntnis muss ebenfalls beide Codepfade erreichen.

Genau das ist mit der Zeit passiert. 2017 “yet third identical fix”. 2018 „apply statistics fix to user statistics". Und zehn Jahre nach der ursprünglichen Abspaltung landet PR #5225: eine SQL Injection in der Statistics-Filterlogik. Der Fix muss in beiden Helpern vorgenommen werden.

Das bedeutet nicht, dass die Duplizierung die SQL Injection verursacht hat. Eine solche Kausalität lässt sich daraus nicht ableiten. Sie erhöht aber die Wartungsoberfläche. Wenn sicherheitskritische Logik zweimal existiert, muss eine Sicherheitskorrektur zweimal gefunden, zweimal korrekt implementiert und zweimal getestet werden. Und mit jeder unabhängigen Änderung steigt die Gefahr, dass beide Varianten nicht mehr exakt dasselbe Verhalten besitzen.

Die technische Schuld besteht damit nicht nur aus zusätzlichen Zeilen Code. Die Zinsen bestehen aus Synchronisationsarbeit.

Drift

Duplizierter Code bleibt selten dauerhaft identisch. Admin Statistics und User Statistics hatten unterschiedliche Anforderungen. Also wurden beide Varianten über die Jahre verändert. Das ist zunächst sogar der Sinn der Abspaltung.

Mit der Zeit entsteht dadurch jedoch ein schwieriger Zustand: Manche Unterschiede sind fachlich notwendig, andere historisch gewachsen. Von außen ist nicht mehr ohne Weiteres erkennbar, welcher Unterschied zu welcher Kategorie gehört.

Damit verändert sich die entscheidende Wartungsfrage. Sie lautet nicht mehr:

Sind diese beiden Funktionen gleich?

Sondern:

Sollten sie gleich sein?

Diese Frage ist erheblich teurer zu beantworten. Denn dafür reicht kein Diff mehr. Man braucht Wissen über die Domäne, die Historie und die unterschiedlichen Einsatzkontexte.

Copy-and-Paste spart zunächst die Kosten einer Abstraktion. Später muss dafür die Bedeutung der Unterschiede rekonstruiert werden.

Zurück ins Jahr 2026

Damit sieht PR #5225 am Ende anders aus als zu Beginn.

Der Pull Request selbst ist kein Beispiel für einen schlechten Security-Prozess. Im Gegenteil. Eine konkrete Schwachstelle wurde identifiziert. Der Fix blieb fokussiert. Im Review wurden weitere problematische Pfade gefunden und korrigiert. QA testete die Änderung. Der PR wurde innerhalb eines Tages gemerged¹.

Genau so sollte ein dringender Security-Fix behandelt werden.

Aber der PR legt gleichzeitig eine ältere strukturelle Schuld frei. Die Allowlist für gültige Response-Spalten musste nicht einmal, sondern zweimal eingebaut werden, weil dieselbe Art von Filter- und Query-Logik in zwei über Jahre getrennt entwickelten Statistics-Helpern existiert.

Der Security-Fix ist damit nicht das Problem. Er ist die Sonde, mit der eine zehn Jahre alte Architekturentscheidung sichtbar wird.

Was nun?

Die falsche Konsequenz wäre wahrscheinlich, beide Dateien in einem großen Refactoring sofort zusammenzuführen.

Nach zehn Jahren unabhängiger Entwicklung weiß niemand ohne genaue Analyse, welche Unterschiede beabsichtigt sind und welche nur historische Drift darstellen. Ein Big-Bang-Refactoring würde genau in einem besonders sensiblen Bereich – Statistik, dynamische Response-Tabellen, Filter und SQL – ein erhebliches Regression-Risiko erzeugen.

Sinnvoller wäre, die gemeinsamen Verantwortlichkeiten schrittweise zu identifizieren. Gerade die Filter- und Query-Erzeugung wäre dafür ein Kandidat. Nicht weil sie besonders elegant zu refactoren wäre, sondern weil PR #5225 gerade gezeigt hat, dass dort duplizierte sicherheitskritische Logik existiert.

Ein gemeinsamer, getesteter Mechanismus zur Interpretation und Validierung von Statistikfiltern würde eine wichtige Eigenschaft herstellen: Eine zukünftige Security-Erkenntnis müsste nur noch an einer Stelle korrekt umgesetzt werden.

Das kleine Fossil

Und damit zurück zu square().

Die Funktion selbst ist bedeutungslos. Sie ist nicht die Ursache der SQL Injection. Sie macht das System nicht langsam. Sie erzeugt vermutlich seit Jahren zuverlässig Quadratzahlen.

Aber sie erzählt eine Geschichte. Dasselbe square() existiert zweimal, weil vor zehn Jahren ein großer Teil eines Statistics-Subsystems einen zweiten Entwicklungszweig bekam. Ein Jahr später dokumentiert die Git-Historie bereits den „third identical fix". Zehn Jahre später muss ein SQL-Injection-Fix wieder durch beide Zweige getragen werden.

Das ist vielleicht die interessanteste Eigenschaft von Technical Debt: Man erkennt sie nicht immer an kompliziertem Code. Manchmal erkennt man sie an zwei völlig korrekten Funktionen.

function square($number)

Zweimal.