Prüfung von Center Token Minting-Verträgen

Die Teams Circle und Coinbase haben uns gebeten, die Münzverträge des Center Token zu überprüfen und zu prüfen. Wir haben uns den Code angesehen und veröffentlichen jetzt unsere Ergebnisse.

Der Code befindet sich im Center-Token-Repository. Die geprüften Dateien sind Controller.sol , MintController.sol und MasterMinter.sol im Verzeichnis Verträge / Münzprägung . Insbesondere haben wir keine anderen Dateien im Center Token-Projekt geprüft oder wie die Münzverträge mit anderen Verträgen darin interagieren. Die für diesen Bericht verwendete Version lautet commit 4b9ebf3941a48e41e7363fee729035610a71ca66 .

Hier finden Sie unsere Einschätzung und Empfehlungen in der Reihenfolge ihrer Wichtigkeit.

Update: Circle- und Coinbase-Teams haben basierend auf unseren Empfehlungen einige Korrekturen vorgenommen. Im Folgenden werden die Korrekturen behandelt, die zum Festschreiben von fbb6cfeb503ece6719cd456f82e6ef1602145db3 eingeführt wurden. Dieses Update umfasst die drei ursprünglich geprüften Dateien sowie die neue MinterManagementInterface.sol Datei, die gemäß unserer Empfehlung von MinController.sol getrennt wurde.

Kritischer Schweregrad

Keine.

Hoher Schweregrad

Keine.

Mittlerer Schweregrad

Fehlende Null-Adressprüfungen

An mehreren Stellen im Code werden Adressen als Parameter an Funktionen übergeben. In vielen dieser Fälle überprüfen die Funktionen nicht, ob die übergebene Adresse nicht die Adresse 0 ist. Im Controller -Vertrag geschieht dies in den Funktionen configureController (die nur _worker prüfen) und removeController .

Im MintController -Vertrag ist andererseits zu erwarten, dass der Konstruktor die Variable minterManager auf 0 setzt, aber die Die Funktion setMinterManager verhindert nicht, dass sie auf 0 gesetzt wird.

Obwohl dies derzeit kein Sicherheitsrisiko darstellt, sollten Sie prüfen, ob die übergebenen Adressen ungleich Null sind, um bei Bedarf unerwartetes Verhalten zu verhindern, oder die Tatsache dokumentieren, dass eine Nulladresse tatsächlich ein gültiger Parameter ist.

Update: Funktionen in Controller überprüfen Sie jetzt alle Argumente , um zu verhindern, dass Nulladressen gesetzt werden. In MintController wurden diesbezüglich keine Änderungen vorgenommen.

Keine Möglichkeit, eine Zulage zu verringern, ohne Minter neu zu konfigurieren

Der MintController -Vertrag bietet einen Mechanismus zum Erhöhen der Toleranz eines Minters, der nur funktioniert, wenn der Minter aktiv ist, und verhindert so die versehentliche Reaktivierung inaktiver Minters und wie in der Inline beschrieben Dokumentation, die Verwendung einer signierten Transaktion zur Reaktivierung eines Minters. Dieser Mechanismus ist in der Funktion incrementMinterAllowance implementiert, die wiederum minterManager.isMinter aufruft, um die erforderliche Prüfung durchzuführen.

Der Vertrag bietet jedoch keinen analogen Mechanismus zum Verringern der Toleranz eines Minters, was bedeutet, dass dies nur durch "Zurücksetzen" des Minters über den configureMinter Funktion.

Es kann durchaus sein, dass dies beabsichtigt ist und dass das Verringern von Minter-Zertifikaten im Kontext keinen Sinn ergibt. Ist dies jedoch nicht der Fall, sollten Sie diese ergänzende Funktionalität bereitstellen.

Update: Die Funktion decrementMinterAllowance wurde eingeführt, um Zertifikatsabnahmen zu handhaben, ohne die Gefahr einer unerwünschten erneuten Aktivierung zu haben . Seine Implementierung ist so, dass, wenn der Wert, um den die Zulage verringert werden soll, größer als die aktuelle Zulage ist, letztere auf Null gesetzt wird. Erwägen Sie, dieses Verhalten inline zu dokumentieren.

Beachten Sie, dass hier keine Überprüfungen durchgeführt werden, ob Zertifikatsabnahmen stattfinden, nachdem Token bereits geprägt wurden, da die eigentliche Münzlogik an anderer Stelle behandelt wird. Die Interaktion zwischen Münzprägung und Zertifikatsverwaltung liegt außerhalb des Rahmens dieser Prüfung. Wenn dies nicht bereits geschehen ist, sollten Sie auch eine vollständige Sicherheitsprüfung für die entsprechenden Verträge durchführen.

Niedriger Schweregrad

Experimentelle Version von Ownable verwendet

Controller.sol importiert eine benutzerdefinierte Version von Ownable , die aus dem Repository labs von ZeppelinOS entnommen und später bearbeitet wurde. Dies ist experimenteller Code und, wie in der README angegeben, nicht für die Produktion bestimmt. Als Randnotiz wird der Import mit import & # x27; ./../ Ownable.sol & # x27; durchgeführt, das einen überflüssigen führenden ./ hat.

Da das Projekt bereits OpenZeppelin verwendet, sollten Sie die bewährte Version von Ownable verwenden.

Mangelnde Kontrolle kann zu Gasverbrennung führen

Die Funktion incrementMinterAllowance kann mit dem Parameterwert allowanceIncrement = 0 aufgerufen werden. Dies würde zu Gasausgaben ohne Zustandsänderungen oder nützliche Berechnungen führen.

Überprüfen Sie, ob allowanceIncrement & gt; 0 , um unnötige Gaskosten zu vermeiden.

Update: Die -IncrementMinterAllowance -Funktion erfordert jetzt dass sein (umbenannter) _allowanceIncrement Parameter größer als 0 ist.

Fehlende Fehlermeldungen in require-Anweisungen

Es gibt mehrere require -Anweisungen, die keine Fehlermeldungen enthalten (Zeilen 43 und 57 in Controller.sol und Zeile 94 in MinterController.sol ). Erwägen Sie, spezifische und informative Fehlermeldungen in alle erfordern -Anweisungen aufzunehmen.

Update: alle erforderlich -Anweisungen stellen jetzt Nachrichten für die Zurücksetzer bereit

Ungeprüfte Version von OpenZeppelin verwendet

Das Repository, in dem die geprüften Verträge aufbewahrt werden, verwendet openzeppelin-solidity v1.11.0 . Dies ist eine veraltete Version und wurde nie geprüft. Zum Zeitpunkt des Schreibens ist die neueste stabile Version v2.0.0 , die eine externe Sicherheitsüberprüfung durchlaufen hat.

Erwägen Sie, das Projekt auf die neuere, geprüfte Version zu aktualisieren.

Verbesserungspotenzial bei der Vertragsdokumentation

Die Münzverträge für Center Token sind im Allgemeinen gut dokumentiert, und die Dokumentation entspricht dem NatSpec-Format. Es gibt jedoch Raum für Verbesserungen. Erstens hat MasterMinter.sol überhaupt keine Dokumentation. Zweitens, während alle Verträge eine kurze Beschreibung jeder Funktion enthalten, verwenden alle Kommentare nur das @dev -Tag unter den vielen in NatSpec verfügbaren.

Ein weiterer Fall, in dem die Dokumentation verbessert werden kann, ist der folgende Kommentar in Controller.sol : "Controller eines bestimmten _workers festlegen". Dies könnte den Benutzer zu der Annahme veranlassen, dass jeder Mitarbeiter einen einzelnen Controller hat, während das System tatsächlich zulässt, dass er mehr als einen hat.

Schließlich scheint sich der Kommentar "erlaubt die Steuerung des Konfigurierens / Entfernens von Minter ..." in MintController auf configureMinter und zu beziehen removeMinter -Funktionen, auf die jedoch nicht mit ihren vollständigen Namen verwiesen wird.

Fügen Sie möglicherweise vollständige docstrings für alle Verträge, Strukturfelder, Statusvariablen, Zuordnungsschlüssel und Funktionen hinzu und lassen Sie Kommentare die Funktionalität der Verträge klarer und genauer darstellen.

Update: Die Inline-Dokumentation wurde erheblich verbessert und deckt insbesondere alle oben genannten Punkte ab. Die Dokumentation verweist in zwei Anlässen auf ein MinterManagerInterface , dessen korrekter Name MinterManagementInterface

ist.

Notizen & amp; Zusätzliche Informationen:

Schlussfolgerung

Es wurden keine kritischen oder schwerwiegenden Probleme gefunden. Es wurden einige Änderungen vorgeschlagen, um bewährten Methoden zu folgen und die potenzielle Angriffsfläche zu verringern.

Wenn Sie an intelligenter Vertragssicherheit interessiert sind, können Sie die Diskussion in unserem Forum fortsetzen, uns auf Medium folgen oder noch besser dem Team beitreten 🚀. Wenn Sie ein eigenes Projekt erstellen und eine Sicherheitsüberprüfung anfordern möchten, tun Sie dies bitte hier.

Beachten Sie, dass die obige Überprüfung zum Zeitpunkt der Veröffentlichung das aktuelle Verständnis bekannter Sicherheitsmuster widerspiegelt, die sich auf die Münzverträge für Center Token beziehen. Das oben Gesagte sollte nicht als Anlageberatung ausgelegt werden. Allgemeine Informationen zur Sicherheit intelligenter Verträge finden Sie in unseren Gedanken