Δεκέμβριος 16, 2021
Εισαγωγή
Η 1inch η ομάδα μας ζήτησε να εξετάσουμε και να ελέγξουμε τους Πρωτόκολλο οριακής παραγγελίας v2 έξυπνα συμβόλαια. Εξετάσαμε τον κώδικα και τώρα δημοσιεύουμε τα αποτελέσματά μας.
Έκταση
Ελέγξαμε τη δέσμευση 4d94eea25e4dac6271bfd703096a5c4a4d899b4a
του 1inch/limit-order-protocol
αποθήκη. Στο πεδίο εφαρμογής ήταν οι ακόλουθες συμβάσεις:
- OrderMixin.sol
- OrderRFQMixin.sol
- PredicateHelper.sol
- RevertReasonParser.sol
- Permitable.sol
- ChainlinkCalculator.sol
- ArgumentsDecoder.sol
- AmountCalculator.sol
- NonceManager.sol
- LimitOrderProtocol.sol
- ImmutableOwner.sol
- InteractiveNotificationReceiver.sol
- AggregatorInterface.sol
- IDaiLikePermit.sol
Όλα τα άλλα αρχεία και κατάλογοι έργων (συμπεριλαμβανομένων των δοκιμών), μαζί με τις εξωτερικές εξαρτήσεις και έργα, τη θεωρία παιχνιδιών και τον σχεδιασμό κινήτρων, εξαιρέθηκαν επίσης από το πεδίο εφαρμογής αυτού του ελέγχου. Οι εξαρτήσεις εξωτερικών κωδικών και συμβολαίων θεωρήθηκε ότι λειτουργούν ως τεκμηριωμένες και οι υπηρεσίες back-end που παρέχονται από 1inch θεωρήθηκε ότι ενεργούν προς το συμφέρον του πρωτοκόλλου.
Συνολική Υγεία
Γενικά, διαπιστώσαμε ότι η βάση κώδικα του έργου είναι ευανάγνωστη και καλά οργανωμένη, αν και θα μπορούσε να επωφεληθεί από πιο εκτεταμένη τεκμηρίωση, ειδικά γύρω από τα μπλοκ κώδικα συναρμολόγησης, τις ακμές περιπτώσεις του πρωτοκόλλου, τα περιουσιακά στοιχεία/κατηγορίες/τους εξωτερικούς πόρους που θα χρησιμοποιηθούν. ευθύνες/περιορισμοί της παρεχόμενης υπηρεσίας υποστήριξης και αλληλεπιδράσεις μεταξύ των παραγόντων. Το έργο καταβάλλει κάθε δυνατή προσπάθεια για να καταστήσει τις ενέργειες αποδοτικές ως προς το αέριο, περιστασιακά, ακόμη και με τον κίνδυνο να καταστήσει τον κώδικα πιο δύσκολο να αιτιολογήσει. θίγουμε ζητήματα που σχετίζονται με αυτό παρακάτω. Καθ' όλη τη διάρκεια του ελέγχου, η ομάδα 1 ιντσών ήταν εξαιρετικά διαθέσιμη, ανταποκρίθηκε και πολύ εύκολη στη συνεργασία.
Επισκόπηση Συστήματος
Το πρωτόκολλο Limit Order ενεργοποιεί την παραγγελία makers
να υπογράφει παραγγελίες εκτός αλυσίδας για ανταλλαγές κουπονιών. Το πρωτόκολλο στη συνέχεια διευκολύνει την πλήρωση παραγγελιών που έχουν υπογραφεί προηγουμένως κατά παραγγελία takers
. Οι παραγγελίες είναι εξαιρετικά επεκτάσιμες και μπορούν να καλέσουν στατικά εξωτερικά συμβόλαια σε πολλά σημεία κατά τη διάρκεια της διαδικασίας πλήρωσης παραγγελιών. Αυτή η επεκτασιμότητα διαποτίζει το πρωτόκολλο με χρησιμότητα, αλλά προσθέτει πολυπλοκότητα και μεγαλύτερη επιφάνεια επίθεσης για τις ίδιες τις παραγγελίες.
Είναι σημαντικό να σημειωθεί ότι δεν υπάρχει αποθήκευση στοιχείων παραγγελίας στην αλυσίδα. Η κατάσταση πλήρωσης ή η κατάσταση ακύρωσης των παραγγελιών παρακολουθείται μόνο μέσω κατακερματισμού παραγγελιών. Αυτό απαιτεί την κοινή χρήση των παραγγελιών peer-to-peer ή μέσω ενός κεντρικού μέρους. Σε αυτήν την περίπτωση, η ομάδα της 1 ίντσας σκοπεύει να ενεργήσει ως αυτό το κεντρικό μέρος, συγκεντρώνοντας υπογεγραμμένες εντολές και χρησιμοποιώντας αυτές τις εντολές ως πηγή ρευστότητας για τα άλλα πρωτόκολλά της. Οι παραγγελίες θα δημοσιεύονται μέσω του δικού τους API, ώστε οι χρήστες να μπορούν να αλληλεπιδρούν μαζί τους.
Αυτός ο συγκεντρωτισμός δίνει στην ομάδα 1 ιντσών ακραίο έλεγχο σχετικά με τις εντολές που δημοσιεύονται και τελικά εκτελούνται. Αυτό τους δίνει επίσης τη δυνατότητα να λογοκρίνουν παραγγελίες, κάτι που θα μπορούσε να είναι χρήσιμο σε περίπτωση κακόβουλων ή παραπλανητικών εντολών, αλλά θα μπορούσε επίσης να γίνει κατάχρηση και να τους επιτρέψει να προπορευθούν οποιονδήποτε άλλο χρήστη σε περίπτωση ευνοϊκής παραγγελίας, μη εμφανίζοντάς το μέσω του API.
Προνομιακοί Ρόλοι
Αν και οι συμβάσεις στις οποίες χρησιμοποιείται ο ρόλος ήταν εκτός πεδίου εφαρμογής, εντοπίστηκε ένας προνομιούχος ρόλος. Ενα immutableOwner
ορίζεται στον δημιουργό μιας σύμβασης μεσολάβησης κατά τη στιγμή της κατασκευής και χρησιμοποιείται για τον περιορισμό της πρόσβασης στο διακομιστή μεσολάβησης external
λειτουργίες.
Εξωτερικές εξαρτήσεις και υποθέσεις εμπιστοσύνης
Ο σχεδιασμός αυτού του πρωτοκόλλου απαιτεί εξαρτήματα εκτός αλυσίδας και εντός αλυσίδας και αυτό το υβριδικό μοντέλο μπορεί να χρησιμοποιηθεί για τον μετριασμό ορισμένων φορέων επίθεσης που προσδιορίζουμε στην έκθεσή μας, αλλά το κόστος αυτής της ικανότητας είναι η αυξημένη εξάρτηση από την ομάδα και την υποδομή 1 ίντσας.
Επιπλέον, το Πρωτόκολλο Limit Order παρέχει λειτουργίες που προορίζονται για την ανάκτηση τιμών από τους χρησμούς Chainlink. Υποθέσαμε ότι αυτοί οι χρησμοί ήταν ειλικρινείς, προσβάσιμοι και λειτουργούσαν σωστά.
Επιπλέον, λόγω της ευελιξίας μιας παραγγελίας, υπάρχουν αρκετά σημεία επαφής με εξωτερικές συμβάσεις που δεν επικυρώνονται. Αυτό σημαίνει ότι ένας κακόβουλος χρήστης θα μπορούσε να κάνει κατάχρηση τέτοιων κλήσεων και να μιμηθεί κατηγόρημα, στοιχεία ή χρησμούς με κακόβουλα συμβόλαια προκειμένου να εκτελέσει ενέργειες κατά την πλήρωση παραγγελιών. Αν και το έργο προστατεύεται σε ορισμένες περιοχές από την επανεισαγωγή, τέτοιοι φορείς θα μπορούσαν να προκαλέσουν επιθέσεις άρνησης υπηρεσίας ή μη ανιχνευμένες παραγγελίες ανεπιθύμητης αλληλογραφίας. Η ομάδα 1 ιντσών γνωρίζει ότι ενδέχεται να εμφανιστούν ορισμένα ζητήματα κατά τη χρήση άγνωστων συμβάσεων για το πρωτόκολλο και έχει δηλώσει την πρόθεσή της ότι μόνο τα κύρια στοιχεία «μπλε τσιπ» θα υποστηρίζονται πλήρως από το έργο. Ωστόσο, θα πρέπει να σημειωθεί ότι ακόμη και με τα πιο δημοφιλή περιουσιακά στοιχεία υπάρχουν εγγενείς συμπεριφορές από κάθε στοιχείο που μπορεί να προκαλέσουν προβλήματα στα πρωτόκολλα που δεν τα αντιμετωπίζουν σωστά, όπως η χρέωση κατά τις μεταφορές με USDT ή η επιστροφή ενός κωδικού σφάλματος αντί για Boolean επιτυχίας με cTokens.
Ευρήματα
Εδώ παρουσιάζουμε τα ευρήματά μας.
Κρίσιμη σοβαρότητα
Καμία.
Υψηλή σοβαρότητα
[H01] Διαβιβάστηκαν ασυνεπή δεδομένα _makeCall
Στο OrderMixin
σύμβαση, το _makeCall
η λειτουργία χρησιμοποιείται για τη μεταφορά περιουσιακών στοιχείων από τον λήπτη στον κατασκευαστή και στη συνέχεια από τον κατασκευαστή στον λήπτη. Στην τελευταία μεταφορά, η _makeCall
η συνάρτηση έχει περάσει εσφαλμένα από την παραγγελία makerAsset
ως τελευταία παράμετρος, όταν θα έπρεπε να είναι της παραγγελίας makerAssetData
.
Ως αποτέλεσμα, οποιαδήποτε λειτουργικότητα διακομιστή μεσολάβησης που βασίζεται στο makerAssetData
το επιχείρημα θα σπάσει.
Για να είναι συνεπής με την προηγούμενη κλήση προς _makeCall
και για να υποστηρίξετε πλήρως τη λειτουργικότητα του διακομιστή μεσολάβησης, εξετάστε το ενδεχόμενο να ενημερώσετε το order.makerAsset
παράμετρος για την order.makerAssetData
.
Ενημέρωση: Διορθώθηκε το τραβήξτε το αίτημα # 57.
[H02] Οι μερικώς συμπληρωμένες ιδιωτικές παραγγελίες μπορούν να εκπληρωθούν από οποιονδήποτε
Το πρωτόκολλο επιτρέπει τη δημιουργία ιδιωτικών και δημόσιων παραγγελιών. Σε ιδιωτικές παραγγελίες, μόνο το allowedSender
διεύθυνση, που καθορίζεται από τον κατασκευαστή κατά τη δημιουργία της παραγγελίας, μπορεί να εκπληρώσει την παραγγελία.
Ωστόσο, στο OrderMixin
σύμβαση, επικύρωση για το allowedSender
διεύθυνση έχει εσφαλμένο εύρος, που σημαίνει ότι αξιολογείται μόνο μέσα στη λογική που χειρίζεται την πρώτη πλήρωση μιας παραγγελίας. Εάν μια ιδιωτική παραγγελία έχει συμπληρωθεί μερικώς, τότε η επιταγή για το allowedSender
Η διεύθυνση δεν είναι πλέον προσβάσιμη και η παραγγελία μπορεί να εκπληρωθεί από οποιονδήποτε.
Για να διευκρινιστεί η πρόθεση σχετικά με το εάν κάποιος χρήστης θα πρέπει να μπορεί να εκπληρώσει μερικώς συμπληρωμένες ιδιωτικές παραγγελίες ή όχι, εξετάστε το ενδεχόμενο να τεκμηριώσετε τον λόγο για την τρέχουσα συμπεριφορά ή να επικυρώσετε την allowedSender
διεύθυνση εκτός του πεδίου εφαρμογής του πρώτου γεμίσματος για να διασφαλιστεί ότι θα επικυρώνεται κάθε φορά που επιχειρείται συμπλήρωση.
Ενημέρωση: Διορθώθηκε το τραβήξτε το αίτημα # 58.
[H03] Ο κακόβουλος κατασκευαστής θα μπορούσε να εκμεταλλευτεί τα μερικά γεμίσματα για να κλέψει τα περιουσιακά στοιχεία του αποδέκτη
Παραγγελίες από το OrderMixin
σύμβαση έχουν τη δυνατότητα μερικής συμπλήρωσης. Για την υποστήριξη μερικών γεμίσεων, το πρωτόκολλο απαιτεί έναν τρόπο υπολογισμού και των δύο πλευρών των ανταλλαγών. Και τα δυο getMakerAmount
και getTakerAmount
Τα πεδία ορίζονται από τον κατασκευαστή της παραγγελίας για αυτόν ακριβώς τον σκοπό.
Κατά την ολοκλήρωση μιας παραγγελίας, οι παραλήπτες πρέπει να παρέχουν είτε το makingAmount
ή η takingAmount
αξίες καθώς και α thresholdAmount
αξία. Υπάρχουν δύο διαφορετικές διαδρομές κωδικού που μπορούν να ληφθούν, με βάση το εάν το makingAmount
ή η takingAmount
παρασχέθηκε.
Το πρώτο είναι όταν το makingAmount
ορίζεται παράμετρος. Θα μπορούσε κολοβός ο makingAmount
αξία και επίσης υπολογίστε το takingAmount
αξία για αυτό. Σε αυτή την κατάσταση, το thresholdAmount
διασφαλίζει ότι το takingAmount
τιμή που λαμβάνεται είναι όχι απροσδόκητα μεγάλο.
Το δεύτερο είναι όταν το takingAmount
ορίζεται παράμετρος. Σε μια τέτοια περίπτωση, θα γίνει υπολογίστε το makingAmount
αξίας, με δυνατότητα περικόπτοντάς το και επανυπολογίζοντας το takingAmount
αξία αν συμβεί αυτό. Σε αυτή την κατάσταση, το thresholdAmount
αξία εξασφαλίζει ότι το makingAmount
η επιστρεφόμενη τιμή είναι όχι απροσδόκητα μικρό.
Υπάρχουν δύο μέθοδοι εκμετάλλευσης, καθεμία μοναδική σε μία από τις προηγούμενες αναφερθείσες διαδρομές κώδικα. Αυτές οι μέθοδοι εκμετάλλευσης απαιτούν κακόβουλο getMakerAmount
και getTakerAmount
λειτουργίες. Μια απλή υλοποίηση αυτών των λειτουργιών θα είχε την ίδια συμπεριφορά AmountCalculator
'μικρό getMakerAmount
και getTakerAmount
λειτουργίες, αλλά με έναν διακόπτη με σκληρό κώδικα που θα τους αναγκάσει να επιστρέψουν μια ελεγχόμενη τιμή από τον εισβολέα όταν χρειάζεται.
Το πρώτο, λιγότερο σοβαρό μοτίβο εκμετάλλευσης περιλαμβάνει την πρώτη διαδρομή κώδικα όπου το makingAmount
η τιμή καθορίζεται σε σειρά συμπλήρωσης. Ένας κακόβουλος κατασκευαστής θα περίμενε μια εντολή συμπλήρωσης που καθορίζει makingAmount
για να εμφανιστεί στο mempool για να το πρωτοπορήσει. Θα στραγγίξουν όλη την αξία εκτός από 1 από την πλευρά του κατασκευαστή και μετά πίεζαν _callGetTakerAmount
για να επιστρέψετε το ποσό που καθορίζεται στο χρήστη thresholdAmount
αξία (ή το επίδομά τους αν είναι μικρότερο). Όταν ολοκληρωθεί τελικά η συναλλαγή του χρήστη, θα ανταλλάξουν την πλήρη τους thresholdAmount
αξίας takerAsset
για μια ενιαία μονάδα του makerAsset
. Αυτή η εκμετάλλευση περιορίζεται από το ποσό που δίνεται από το thresholdAmount
αξία ή το ποσό του takerAsset
ο χρήστης επέτρεψε στο LimitOrderProtocol
σύμβαση.
Το δεύτερο, πιο σοβαρό μοτίβο εκμετάλλευσης περιλαμβάνει τη δεύτερη διαδρομή κώδικα όπου το takingAmount
η τιμή καθορίζεται. Ο κακόβουλος κατασκευαστής θα περίμενε παρομοίως για μια εντολή συμπλήρωσης που καθόριζε α takingAmount
τιμή για να εμφανιστεί στο mempool. Θα προηγουμένως τη συναλλαγή και θα αναγκάσουν το makingAmount
αξία που επιστράφηκε από _callGetMakerAmount
συνάρτηση να είναι υψηλότερη και από τις δύο remainingMakerAmount
και την thresholdAmount
. Θα έβαζαν επίσης το takingAmount
επιστρεφόμενη τιμή από _callGetTakerAmount
να είναι το ποσό των takerAsset
περιουσιακό στοιχείο που επιτρέπεται στο LimitOrderProtocol
από τον λήπτη. Όταν ολοκληρωθεί η συναλλαγή του λήπτη, θα γίνει περικόβω το makingAmount
αξία και μετά επανυπολογίστε το takingAmount
αξία. Ωστόσο, αυτός ο επανυπολογισμός δεν είναι εγγυημένος ότι θα είναι χαμηλότερος, και σε αυτήν την περίπτωση θα εξαντλήσει τον αποδέκτη όλων των takerAsset
που επέτρεψαν στη σύμβαση. Σε αυτή τη διαδρομή κωδικού, το thresholdAmount
η τιμή είναι διασφαλίζοντας ότι το makingAmount
δεν είναι πολύ χαμηλό, οπότε παίρνοντας όλα τα του λήπτη takerAsset
Το στοιχείο δεν έχει επιλεγεί. Τα κεφάλαια που χάνονται δεσμεύονται από το ποσό του takerAsset
περιουσιακό στοιχείο που επέτρεψε ο χρήστης στο LimitOrderProtocol
σύμβαση.
Αυτά τα exploits δεν είναι δυνατά χωρίς μερικές παραγγελίες και, πιο συγκεκριμένα, μερικές παραγγελίες με κακόβουλες getMakerAmount
και getTakerAmount
υλοποιήσεις.
Το κύριο θέμα του thresholdAmount
Ο έλεγχος αξίας είναι ότι καλύπτει μόνο τη μία πλευρά της ανταλλαγής, αλλά η άλλη πλευρά μπορεί να χειραγωγηθεί μέσω του frontrunning. Δεν υπάρχουν διαβεβαιώσεις ότι η αξία που πρότεινε αρχικά ο λήπτης παραμένει αμετάβλητη. Εξετάστε το ενδεχόμενο κατάργησης makingAmount
περικοπή και από τις δύο διαδρομές κώδικα και επαναφορά εάν η παραγγελία δεν μπορεί να υποστηρίξει ένα γέμισμα τόσο μεγάλο όσο ζητήθηκε. Κάνοντας αυτό, το thresholdAmount
μπορεί να χρησιμοποιηθεί για να περιορίσει επαρκώς την άλλη πλευρά της ανταλλαγής και να αποφύγει την απροσδόκητη συμπεριφορά, ακόμη και σε κακόβουλες εντολές.
Ενημέρωση: Διορθώθηκε το τραβήξτε το αίτημα # 83.
Μέση σοβαρότητα
[M01] Στατικά ορίσματα μεταβιβάζονται μετά από δυναμικά ορίσματα
Στο OrderMixin
σύμβαση, το getTakerAmount
και getMakerAmount
Τα πεδία bytes χρησιμοποιούνται ως ορίσματα για το _callGetTakerAmount
και _callGetMakerAmount
λειτουργίες. Αυτές οι κλήσεις παρέχουν έναν τρόπο υπολογισμού της μίας πλευράς της ανταλλαγής με βάση την άλλη πλευρά και επιτρέπουν στους χρήστες να συμπληρώσουν εν μέρει τις παραγγελίες.
Η getTakerAmount
/getMakerAmount
Τα πεδία είναι δυναμικές μεταβλητές και συσκευάζονται μπροστά από το takerAmount
και makerAmount
τιμές στο _callGetTakerAmount
και _callGetMakerAmount
λειτουργίες. Είναι δυνατό για έναν κακόβουλο κατασκευαστή να παρέχει περισσότερα δεδομένα από τα αναμενόμενα στο getTakerAmount
και getMakerAmount
πεδία για να ωθήσει το takerAmount
και makerAmount
byte πέρα από εκεί που υποτίθεται ότι βρίσκονται κατά την αποκωδικοποίηση στην επόμενη συνάρτηση. Αυτό επιτρέπει στον κατασκευαστή να μετατοπίσει το ποσό που έχει διαβιβαστεί στον παραλήπτη ή τον κατασκευαστή κατά ένα πλήρες byte προς τα δεξιά και ακόμη και να τα αντικαταστήσει πλήρως εάν παρέχονται επιπλέον 32 byte δεδομένων.
Οι χρήστες πρέπει ήδη να ελέγξουν με μη αυτόματο τρόπο το getTakerAmount
και getMakerAmount
πεδία με τη σειρά, αλλά αυτή η τεχνική είναι μάλλον δύσκολο να εντοπιστεί. Αξίζει επίσης να σημειωθεί ότι αυτή η επίθεση ισχύει ακόμη και για τους εσωτερικά έμπιστους getMakerAmount
και getTakerAmount
λειτουργίες. Για τις περισσότερες επιθέσεις, η παροχή ενός λογικού ορίου θα αποτρέψει την απώλεια κεφαλαίων.
Για να αποφευχθεί αυτό, εξετάστε το ενδεχόμενο να κωδικοποιήσετε τα στατικά ορίσματα πριν από τα δυναμικά ορίσματα για να αποφύγετε να δώσετε στα δυναμικά ορίσματα μια μέθοδο ελέγχου των στατικών ορισμάτων.
Ενημέρωση: Δεν φτιάχτηκε. Η ομάδα της 1 ίντσας δήλωσε:
Θα δείξουμε ιδιαίτερη προσοχή με την επικύρωση του λήπτη. Θα προσπαθήσουμε να εφαρμόσουμε την ορθολογική επικύρωση των ληπτών στο sdk μας που θα βοηθήσει στο φιλτράρισμα δυνητικά κακόβουλων παραγγελιών.
[M02] Οι παραγγελίες ERC721 μπορούν να χειριστούν
Είναι δυνατή η ανταλλαγή περισσότερων από μόνο ERC20 μέσω του OrderMixin
με την ανάπτυξη μιας σύμβασης που μοιράζεται τον ίδιο επιλογέα λειτουργιών με το IERC20 transferFrom
, και παρέχοντας τη σύμβαση ως το makerAsset
ή η takerAsset
σε μια παραγγελία.
Οι πληρεξούσιοι εκτός πεδίου εφαρμογής, συγκεκριμένα, ERC721Proxy
, ERC721ProxySafe
, να ERC1155Proxy
οι συμβάσεις ακολουθούν αυτό το πρότυπο για να παρέχουν υποστήριξη ERC721
και ERC1155
μάρκες. Δεδομένου ότι οι διακομιστής μεσολάβησης πρέπει να καλούνται με το ίδιο μοτίβο με ένα IERC20 transferFrom
κλήση, η υπογραφή πρέπει να ξεκινά με address from
, address to
και uint256 amount
. Οτιδήποτε άλλο απαιτούν οι διακομιστής μεσολάβησης μπορεί να μεταβιβαστεί μετά και ορίζεται με τη σειρά ως makerAssetData
και takerAssetData
.
Τα ERC1155 μπορούν φυσικά να μεταφέρουν πολλαπλά από τα ίδια αναγνωριστικά διακριτικά ταυτόχρονα, πράγμα που σημαίνει ότι ERC1155Proxy
σύμβαση κάνει χρήση του amount
πεδίο. Αφ 'ετέρου, ERC721
s δεν έχουν προφανή χρήση για το amount
πεδίο. Δεδομένου ότι αντιπροσωπεύουν μη ανταλλάξιμα token, ένα συγκεκριμένο tokenId θα έχει μόνο ένα σε ύπαρξη, αποδίδοντας το amount
χωράφι άχρηστο. Εξαιτίας αυτού, η εφαρμογή και για τα δύο ERC721Proxy
και ERC721ProxySafe
συμβάσεις χρησιμοποιούν τα απαιτούμενα amount
πεδίο ως το tokenId
Αντιθέτως.
Αυτή η υπερφόρτωση του amount
παράμετρος δημιουργεί τη δυνατότητα μερικής πλήρωσης ERC721
παραγγελίες για να αγοράσετε ξεχωριστά μάρκες σε μειωμένες τιμές. Για παράδειγμα, θα μπορούσε να υπάρχει μια περίπτωση όπου ένας μεμονωμένος χρήστης έχει πολλαπλούς ERC721
της ίδιας σύμβασης επιτρέπεται να μεταβιβαστούν από την ERC721Proxy
σύμβαση και τα απαριθμεί σε ξεχωριστές οριακές εντολές.
Εάν οι οριακές εντολές παρέχουν επίσης το getMakerAmount
και getTakerAmount
πεδία, θα είναι δυνατή η μερική συμπλήρωσή τους ERC721
παραγγελίες. Από την παραγγελία amount
Το πεδίο στην πραγματικότητα αντιστοιχεί στο tokenId
, ένας κακόβουλος χρήστης μπορεί να τοποθετήσει μερική συμπλήρωση στο ERC721
με το υψηλότερο tokenId, με αποτέλεσμα α makingAmount
/takingAmount
από ένα ERC721
που θα μπορούσε να αντιστοιχεί σε χαμηλότερο tokenId
. Το αποτέλεσμα είναι το ERC721
με το χαμηλότερο tokenId
θα μεταβιβαζόταν στην τιμή του (higher tokenId price) * (lower tokenId's id) / (higher tokenId's id)
.
Αυτό το exploit έχει μερικές απαιτήσεις:
- Πολλαπλούς
ERC721
s από την ίδια σύμβαση να επιτρέπεται σε κάθε έναERC721
πληρεξούσιο από έναν μόνο ιδιοκτήτη. - Ανοιχτή παραγγελία για ένα από τα
ERC721
αυτό δεν είναι το χαμηλότεροtokenId
από τα επιτρεπόμενα. - Επιτρέπονται μερικές γεμίσεις κατά την παραγγελία.
Για να αφαιρέσετε εντελώς την πιθανότητα μερικής ERC721
γεμίζει, σκεφτείτε να διαχωρίσετε το amount
και tokenId
επιχειρήματα. Είτε τα επιχειρήματα είναι διαχωρισμένα είτε όχι, εξετάστε το ενδεχόμενο να το τεκμηριώσετε για να ειδοποιήσετε τους χρήστες για αυτήν τη συμπεριφορά και να αποφύγετε αυτό το μοτίβο στο μέλλον.
Ενημέρωση: Διορθώθηκε το τραβήξτε το αίτημα # 59.
[M03] Μη τεκμηριωμένες δεκαδικές παραδοχές
Η LimitOrderProtocol
συμβόλαιο κληρονομεί το ChainlinkCalculator
σύμβαση μέσω του OrderMixin
σύμβαση. Αυτή η σύμβαση εκθέτει δύο λειτουργίες για να επιτρέψει τη χρήση των χρησμών Chainlink κατά τη διάρκεια του έλεγχος κατηγορημάτων και η αναζήτηση του ποσό κατασκευαστή/ποσό του παραλήπτη.
Ωστόσο, το συμβόλαιο κάνει μη τεκμηριωμένες υποθέσεις σχετικά με τον αριθμό των δεκαδικών που πρέπει να αναφέρουν οι χρησμοί του Chainlink, καθώς και τον αριθμό των δεκαδικών που πρέπει να περιέχουν οι παράμετροι συνάρτησης. Σε ορισμένα σενάρια, αυτό θα μπορούσε να οδηγήσει σε απροσδόκητες συμπεριφορές, συμπεριλαμβανομένης της εσφαλμένης τιμολόγησης περιουσιακών στοιχείων και της ακούσιας απώλειας κεφαλαίων.
Πιο συγκεκριμένα, σε όλη τη διάρκεια του συμβολαίου, η σιωπηρή υπόθεση είναι ότι οι χρησμοί του Chainlink θα αναφέρουν με 18 δεκαδικά ψηφία ακρίβειας. Ωστόσο, όχι όλους τους χρησμούς Chainlink αναφορά με αυτόν τον αριθμό δεκαδικών. Στην πραγματικότητα, εάν το μαντείο αναφέρει ένα ζεύγος κουπονιών που είναι σε όρους νομίσματος (USD, για παράδειγμα), θα έχει μόνο 8 δεκαδικά ψηφία ακρίβειας. Δεδομένου ότι δεν υπάρχουν περιορισμοί στο Ποιό μπορούν να χρησιμοποιηθούν χρησμοί, δεν πρέπει να γίνονται σιωπηρές υποθέσεις σχετικά με τον αριθμό των δεκαδικών με τους οποίους θα αναφέρουν.
Σχετικά, υπάρχει μια σιωπηρή υπόθεση ότι το amount
παράμετρος για το ChainlinkCalculator
Οι συναρτήσεις θα χρησιμοποιούν 18 δεκαδικά ψηφία, μαζί με την παραπλανητική ρητή δήλωση ότι το singlePrice
λειτουργία Calculates price of token relative to ETH scaled by 1e18
. Στην πραγματικότητα, ακόμη και με χρησμό που κάνει αναφορά με 18 δεκαδικά ψηφία, η επιστρεφόμενη τιμή του singlePrice
η συνάρτηση θα κλιμακωθεί με τον αριθμό των δεκαδικών του amount
παράμετρο, η οποία μπορεί να μην είναι απαραίτητα 18 δεκαδικά.
Ομοίως, το doublePrice
Η συνάρτηση υποθέτει ότι δύο χρησμοί Chainlink θα αναφέρουν με τον ίδιο αριθμό δεκαδικών, με αποτέλεσμα το αποτέλεσμα της συνάρτησης να αποκλίνει από τις προσδοκίες.
Εξετάστε το ενδεχόμενο να τεκμηριώσετε ρητά υποθέσεις σχετικά με τον αριθμό των δεκαδικών με τους οποίους πρέπει να είναι οι παράμετροι και οι επιστρεφόμενες τιμές. Επιπλέον, εξετάστε το ενδεχόμενο είτε να περιορίσετε τους υπολογισμούς που εξαρτώνται από χρησμούς που παραβιάζουν αυτές τις υποθέσεις είτε να λάβετε υπόψη τους σχετικούς υπολογισμούς τον πραγματικό αριθμό των δεκαδικών.
Ενημέρωση: Διορθώθηκε το τραβήξτε το αίτημα # 75.
Χαμηλή σοβαρότητα
[L01] Σταθερές που δεν δηλώνονται ρητά
Υπάρχουν μερικές περιπτώσεις κυριολεκτικών τιμών που χρησιμοποιούνται με ανεξήγητο νόημα στη βάση κώδικα. Για παράδειγμα:
- Στο
OrderMixin
σύμβαση, το_remaining
Η χαρτογράφηση είναι σημασιολογικά υπερφορτωμένη (όπως εξηγείται στο τεύχος Σημασιολογική υπερφόρτωση της χαρτογράφησης) για παρακολούθηση του ποσού του περιουσιακού στοιχείου που απομένει για μια μερικώς συμπληρωμένη παραγγελία καθώς εάν μια παραγγελία έχει ολοκληρωθεί. ΕΙΔΙΚΑ,0
σημαίνει ότι δεν έχουν γίνει γεμίσματα που σχετίζονται με μια παραγγελία,1
σημαίνει ότι μια παραγγελία δεν μπορεί πλέον να εκπληρωθεί, και οτιδήποτε μεγαλύτερο από1
σημαίνει ότι υπάρχει ένα υπόλοιπο ποσό που σχετίζεται με την παραγγελία που μπορεί ενδεχομένως να εκπληρωθεί. - Στο
ChainlinkCalculator
σύμβαση, η κυριολεκτική αξία1e18
χρησιμοποιείται στοsinglePrice
λειτουργία.
Για να βελτιώσετε την αναγνωσιμότητα του κώδικα και να διευκολύνετε την ανακατασκευή, σκεφτείτε να ορίσετε μια σταθερά για κάθε μαγικό αριθμό, δίνοντάς του ένα σαφές και αυτονόητο όνομα. Για σύνθετες τιμές, εξετάστε το ενδεχόμενο να προσθέσετε ένα ενσωματωμένο σχόλιο που να εξηγεί πώς υπολογίστηκαν ή γιατί επιλέχθηκαν.
Ενημέρωση: Διορθώθηκε το τραβήξτε το αίτημα # 75 και τραβήξτε το αίτημα # 76.
[L02] Τα κακόβουλα μέρη θα μπορούσαν να εμποδίσουν την εκτέλεση επιτρεπόμενων εντολών
Η OrderMixin
σύμβαση επιτρέπει στους χρήστες του κατασκευαστή να υποβάλλουν επιτρεπόμενες παραγγελίες ώστε να μπορούν να εκτελεστούν σε μία συναλλαγή, αντί να χρειάζεται να υπάρχει ξεχωριστή συναλλαγή για εγκρίσεις. Επίσης, οι παραλήπτες μπορούν υποβάλλουν τη δική τους άδεια κατά την πλήρωση της παραγγελίας για τον ίδιο σκοπό.
Ωστόσο, επειδή η άδεια του κατασκευαστή περιέχεται μέσα στο τάξη, τόσο οι άδειες του κατασκευαστή όσο και του παραλήπτη θα είναι προσβάσιμες όσο η συναλλαγή συμπλήρωσης παραγγελίας βρίσκεται στο mempool. Αυτό θα επέτρεπε σε οποιονδήποτε κακόβουλο χρήστη να λάβει αυτές τις άδειες και να τις εκτελέσει στα αντίστοιχα συμβόλαια περιουσιακών στοιχείων κατά την εκ των προτέρων εκτέλεση της συναλλαγής συμπλήρωσης. Επειδή οι άδειες αυτές έχουν α nonce
για να αποφευχθεί μια επίθεση διπλής δαπάνης, η συναλλαγή συμπλήρωσης της παραγγελίας θα αποτύγχανε ως αποτέλεσμα της προσπάθειας χρήσης της ίδιας άδειας που χρησιμοποιήθηκε μόλις κατά τη διάρκεια της πρώτης εκτέλεσης.
Παρόλο που δεν υπάρχει κίνδυνος ασφάλειας και ο κατασκευαστής θα μπορούσε να δημιουργήσει μια νέα παραγγελία και να προεγκρίνει τη συναλλαγή, αυτή η επίθεση θα μπορούσε σίγουρα να επηρεάσει τη χρηστικότητα των επιτρεπόμενων παραγγελιών. Πράγματι, ένας παρακινημένος επιθετικός θα μπορούσε να μπλοκάρει όλοι επιτρεπόμενες εντολές με αυτή την επίθεση. Εξετάστε το ενδεχόμενο να επικυρώσετε εάν η άδεια έχει ήδη υποβληθεί ή εάν το επίδομα είναι αρκετό, κατά τη διάρκεια της πλήρωσης της παραγγελίας. Εξετάστε επίσης το ενδεχόμενο να ενημερώσετε τους χρήστες σχετικά με αυτήν την πιθανή επίθεση κατά τη σύνθεση της παραγγελίας.
Ενημέρωση: Δεν φτιάχτηκε. Η ομάδα 1 ιντσών αναφέρει:
Είχαμε προηγουμένως ελέγχους έγκρισης, αλλά αποφασίσαμε να απλοποιήσουμε τη ροή αδειών για να επαναφέρουμε απλώς τις ανεπιτυχείς εγκρίσεις. Θα σκεφτούμε τρόπους για να ειδοποιήσουμε τους κατασκευαστές για το ζήτημα.
[L03] Διπλότυπος κωδικός
Υπάρχουν περιπτώσεις διπλότυπου κώδικα στη βάση κώδικα. Η αντιγραφή κώδικα μπορεί να οδηγήσει σε προβλήματα αργότερα στον κύκλο ζωής της ανάπτυξης και αφήνει το έργο πιο επιρρεπές στην εισαγωγή σφαλμάτων. Τέτοια σφάλματα μπορούν να εισαχθούν ακούσια όταν οι αλλαγές λειτουργικότητας δεν αναπαράγονται σε όλες τις παρουσίες κώδικα που θα πρέπει να είναι πανομοιότυπες. Παραδείγματα διπλών κωδικών περιλαμβάνουν:
Αντί να αντιγράψετε κώδικα, σκεφτείτε να έχετε μόνο ένα συμβόλαιο ή βιβλιοθήκη που περιέχει τον διπλότυπο κώδικα και να τον χρησιμοποιείτε όποτε απαιτείται η διπλότυπη λειτουργικότητα.
Ενημέρωση: Εν μέρει σταθερό σε τραβήξτε το αίτημα # 60.
[L04] Εσφαλμένη ή παραπλανητική σειρά δοκιμών
Υπάρχουν περιπτώσεις στη σουίτα δοκιμών όπου τα τεστ αποκλίνουν από την αναμενόμενη συμπεριφορά τους. Για παράδειγμα:
- Η
ChainlinkCalculator
σύμβαση κληρονομείται από τονOrderMixin
σύμβαση. Ωστόσο, κατά τη διάρκεια των δοκιμών τοAmountCalculator.arbitraryStaticCall
η συνάρτηση χρησιμοποιείται για την κλήση τουChainlinkCalculator
σύμβαση ως εξωτερική, ανεξάρτητη σύμβαση. Παρόλο που το αποτέλεσμα είναι το αναμενόμενο, η δοκιμή θα πρέπει να αντικατοπτρίζει τη συμπεριφορά με τον τρέχοντα σχεδιασμό του συστήματος και την αναμενόμενη περίπτωση χρήσης καλώνταςChainlinkCalculator
λειτουργεί απευθείας χωρίς τη χρήση της αυθαίρετης στατικής κλήσης. - Αν και τα συμβόλαια μεσολάβησης ήταν εκτός πεδίου εφαρμογής, παρατηρήσαμε ότι κατά τη δοκιμή του πρωτοκόλλου με στοιχεία ERC721, το
ERC721Proxy
το συμβόλαιο δεν χρησιμοποιείται για την ανταλλαγή των περιουσιακών στοιχείων του δοκιμαστική σουίτα.
Καθώς η ίδια η ομάδα δοκιμών βρίσκεται εκτός του πεδίου αυτού του ελέγχου, εξετάστε το ενδεχόμενο να ελέγξετε διεξοδικά τη δοκιμαστική σουίτα για να βεβαιωθείτε ότι όλες οι δοκιμές εκτελούνται με επιτυχία σύμφωνα με τις προδιαγραφές του πρωτοκόλλου.
Ενημέρωση: Διορθώθηκε το τραβήξτε το αίτημα # 57, τραβήξτε το αίτημα # 59, να τραβήξτε το αίτημα # 61.
[L05] Λάθη και παραλείψεις σε συμβάντα
Σε όλη τη βάση κώδικα, τα συμβάντα γενικά εκπέμπονται όταν γίνονται ευαίσθητες αλλαγές στα συμβόλαια. Ωστόσο, σε πολλά συμβάντα δεν υπάρχουν παράμετροι με ευρετήριο ή/και λείπουν σημαντικές παράμετροι. Για παράδειγμα:
Υπάρχουν επίσης ευαίσθητες ενέργειες στις οποίες λείπουν γεγονότα, όπως:
Εξετάστε το ενδεχόμενο πιο ολοκληρωμένης ευρετηρίασης υπαρχόντων συμβάντων και προσθήκης νέων παραμέτρων όπου λείπουν. Επίσης, εξετάστε το ενδεχόμενο να εκπέμπετε όλα τα συμβάντα με τέτοιο τρόπο ώστε να μπορούν να χρησιμοποιηθούν για την ανοικοδόμηση της κατάστασης της σύμβασης από υπηρεσίες εκτός αλυσίδας.
Ενημέρωση: Δεν φτιάχτηκε. Ωστόσο, η ομάδα 1 ιντσών πρόσθεσε ένα orderRemaining
παράμετρος στο OrderCanceled
εκδήλωση στο τραβήξτε το αίτημα # 62.
Η ομάδα 1 ιντσών αναφέρει:
Διαπιστώσαμε ότι απαιτείται μόνο ένα περιορισμένο υποσύνολο δεδομένων για την ικανοποίηση των αναγκών του frontend. Σε περίπτωση εκτενούς ανάλυσης, όλα τα προτεινόμενα πεδία είναι διαθέσιμα μέσω ιχνηλάτησης. Για
OrderRFQMixin
Αναμένουμε από τους διαπραγματευτές να δημιουργήσουν τον δικό τους εξελιγμένο τρόπο παρακολούθησης των παραγγελιών που έχουν ακυρωθεί.
[L06] Αλλαγές αποθήκευσης κατά την εκπομπή συμβάντος
Στο NonceManager
σύμβαση, όταν το NonceIncreased
εκπέμπεται το συμβάν, αυξάνεται επίσης η μη ύπαρξη του αποστολέα του μηνύματος.
Η ταυτόχρονη εκτέλεση πολλαπλών λειτουργιών μπορεί να κάνει τη βάση κώδικα πιο δύσκολη στην αιτιολογία, πιο επιρρεπή σε σφάλματα και μπορεί να οδηγήσει σε παράβλεψη ή παρανόηση λειτουργιών.
Για να βελτιώσετε τη συνολική πρόθεση, την αναγνωσιμότητα και τη σαφήνεια του κώδικα, εξετάστε το ενδεχόμενο να αυξήσετε την τιμή nonce πριν από την εκπομπή του συμβάντος.
Ενημέρωση: Διορθώθηκε το τραβήξτε το αίτημα # 63.
[L07] Ασυνεπείς μεθοδολογίες αποκωδικοποίησης θα μπορούσαν να προκαλέσουν αποκλίσεις στα αποτελέσματα
Για να υποστηρίξει όλη την επεκτασιμότητα και την ευελιξία του, το Πρωτόκολλο Οριακής Παραγγελίας πρέπει να αντιμετωπίζει τακτικά δεδομένα δυναμικών byte και αυθαίρετες τιμές επιστροφής από εξωτερικά συμβόλαια. Ως αποτέλεσμα, το πρωτόκολλο περιλαμβάνει ένα ArgumentsDecoder
βιβλιοθήκη για πιο αποτελεσματική μετατροπή των τιμών δυναμικών byte σε βασικούς τύπους δεδομένων. Ωστόσο, αυτή η βιβλιοθήκη δεν χρησιμοποιείται αποκλειστικά και σε ορισμένες περιπτώσεις abi.decode
χρησιμοποιείται αντ' αυτού. Επιπλέον, ορισμένα συμβόλαια χρησιμοποιούν abi coder v1
ενώ άλλοι χρησιμοποιούν abi coder v2
. Το πρώτο αποδίδει πιο παρόμοια με το ArgumentsDecoder
βιβλιοθήκη, ενώ η τελευταία εκτελεί πρόσθετους ελέγχους κατά την αποκωδικοποίηση.
Η ασυνεπής χρήση αυτών των διαφορετικών μεθοδολογιών αποκωδικοποίησης μπορεί να οδηγήσει σε ανεπαίσθητες αποκλίσεις μεταξύ της πρόθεσης και της πραγματικής συμπεριφοράς της βάσης κωδικών.
Για παράδειγμα, η simulateCalls
η λειτουργία χρησιμοποιεί μόνο το ArgumentsDecoder.decodeBool
λειτουργία. Εάν το simulateCalls
Η συνάρτηση χρησιμοποιείται για τον έλεγχο κλήσεων που θα γίνονταν στο κατηγόρημα μιας παραγγελίας, τότε τα αποτελέσματά της θα μπορούσαν να αποκλίνουν από αυτό που συμβαίνει στην πραγματικότητα όταν αξιολογούνται οι συνθήκες κατηγόρησης, επειδή χρησιμοποιούνται διαφορετικές μεθοδολογίες αποκωδικοποίησης.
Έτσι, για παράδειγμα, εάν ένα κατηγόρημα κάνει ένα εξωτερικό staticcall
σε κάποια συνάρτηση που επιστρέφει a uint256
τιμή μεγαλύτερη από μία από την αναμενόμενη bool
, τότε αυτή η κλήση θα επανέλθει, επειδή η τιμή επιστροφής είναι αποκωδικοποιημένα με abi coder v2
'μικρό abi.decode
που δεν θα δεχτεί τέτοιες αξίες όπως bool
. Ωστόσο, αν γίνει ακριβώς η ίδια κλήση με simulateCalls
, τότε αυτό απλά θα επισημανθεί ως true
, Επειδή decodeBool
αντιμετωπίζει οποιαδήποτε τιμή μεγαλύτερη από το μηδέν ως true
.
Για να κάνετε το simulateCalls
Η λειτουργία αντικατοπτρίζει πλήρως τη συμπεριφορά των πραγματικών κλήσεων κατηγορήματος, εξετάστε το ενδεχόμενο να την τροποποιήσετε για χρήση abi.decode
.
Ενημέρωση: Διορθώθηκε το τραβήξτε το αίτημα # 82.
[L08] Έλλειψη επικύρωσης εισόδου
Η fillOrderToWithPermit
και fillOrderTo
λειτουργίες του OrderMixin
σύμβαση, καθώς και η fillOrderRFQToWithPermit
και fillOrderRFQTo
λειτουργίες του OrderRFQMixin
συμβόλαιο, μην επικυρώσετε το target
παράμετρος διεύθυνσης.
Αυτό δίνει τη δυνατότητα σε έναν χρήστη να περάσει κατά λάθος τη μηδενική διεύθυνση και, ως εκ τούτου, να κλειδώσει τα περιουσιακά στοιχεία που πρόκειται να λάβει μετά την εκπλήρωση μιας παραγγελίας.
Για να διασφαλίσετε ότι οι χρήστες δεν θα κλειδώσουν κατά λάθος τα χρήματά τους, εξετάστε το ενδεχόμενο να επικυρώσετε ότι το target
διεύθυνση δεν ισούται με τη μηδενική διεύθυνση στις αναφερόμενες συναρτήσεις.
Ενημέρωση: Διορθώθηκε το τραβήξτε το αίτημα # 78.
[L09] Χαμηλή κάλυψη δοκιμής μονάδας
Η κάλυψη δοκιμής μονάδας για ολόκληρο το έργο είναι περίπου 75%, με ορισμένες από τις συμβάσεις να έχουν ιδιαίτερα χαμηλή κάλυψη.
Λαμβάνοντας υπόψη τη σημασία των δοκιμών μονάδας για την επικύρωση κώδικα και την αποτροπή παλινδρόμησης κατά την αναδιαμόρφωση και την ανάπτυξη νέων λειτουργιών, ενθαρρύνουμε τη σημαντική αύξηση της κάλυψης δοκιμής μονάδας σε τουλάχιστον 95%, συμπεριλαμβανομένων περιπτώσεων ακμών που καλύπτουν ακόμη και απίθανες καταστάσεις.
Ενημέρωση: Δεν φτιάχτηκε.
[L10] Παραπλανητική ή ελλιπής ενσωματωμένη τεκμηρίωση
Σε όλη τη βάση κωδικών, εντοπίστηκαν μερικές περιπτώσεις παραπλανητικής ή/και ελλιπούς ενσωματωμένης τεκμηρίωσης και πρέπει να διορθωθούν.
Τα ακόλουθα είναι περιπτώσεις παραπλανητικής ενσωματωμένης τεκμηρίωσης:
- Στο
ChainlinkCalculator
σύμβαση, τοsinglePrice
λειτουργία NatSpec@notice
ετικέτα λέει ότιCalculates price of token relative to ETH scaled by 1e18
, αλλά στην πραγματικότητα, το αποτέλεσμά του είναι το αξία ofamount
κουπόνια σε κλίμακα1e18
, όπου το μαντείο μπορεί να μην αναφέρει ως προς το ETH (για παράδειγμα, για ένα ζευγάρι που δεν περιλαμβάνει ETH). - Στο
OrderRFQMixin
σύμβαση, τοinvalidatorForOrderRFQ
λειτουργία NatSpec@return
ετικέτα είναι παραπλανητικό, επειδή η προσφορά μπορεί να μην έχει συμπληρωθεί για να έχει οριστεί το αντίστοιχο bit ακυρότητας. Η παραγγελία μπορεί επίσης να έχει ακυρωθεί. - Στις γραμμές 147, 165, να 188 of
OrderMixin.sol
, το NatSpec@param
Οι ετικέτες δεν είναι γραμματικές. - On line 20 of
ERC1155Proxy.sol
, τη@notice
Η ετικέτα δηλώνει ότι ο υπολογισμένος κατακερματισμός είναι το αποτέλεσμα του κατακερματισμού τουfunc_733NCGU
λειτουργία, όπου θα έπρεπε να είναι ηfunc_301JL5R
αντ 'αυτού.
Τα ακόλουθα είναι περιπτώσεις ελλιπούς ενσωματωμένης τεκμηρίωσης:
- Λειτουργίες στο
AmountCalculator
σύμβαση δεν περιγράφουν καμία από τις παραμέτρους. - Στο
ChainlinkCalculator
σύμβαση, τοsinglePrice
καιdoublePrice
Οι συναρτήσεις δεν περιγράφουν όλες τις παραμέτρους. - Στο
ImmutableOwner
σύμβαση, η δημόσια μεταβλητή και ο τροποποιητής δεν έχουν NatSpec. - Στο
InteractiveNotificationReceiver
σύμβαση, τοnotifyFillOrder
η συνάρτηση δεν περιγράφει καμία από τις παραμέτρους. - Στο
LimitOrderProtocol
σύμβαση, τοDOMAIN_SEPARATOR
η λειτουργία δεν έχει NatSpec. - Γεγονότα και χαρτογραφήσεις στο
NonceManager
δεν έχουν NatSpec. - Στο
OrderRFQMixin
σύμβαση,cancelOrderRFQ*
Οι συναρτήσεις δεν περιγράφουν τις επιστρεφόμενες τιμές. - Στο
OrderMixin
σύμβαση, πολλές λειτουργίες δεν έχουν πλήρες NatSpec. - On line 168 of
OrderMixin.sol
και on line 71 ofOrderRFQMixin.sol
, λείπει το@dev
tags. - Λειτουργίες στο
PredicateHelper
συμβόλαιο δεν περιγράφουν όλες τις παραμέτρους.
Η σαφής ενσωματωμένη τεκμηρίωση είναι θεμελιώδης για την περιγραφή των προθέσεων του κώδικα. Οι αναντιστοιχίες μεταξύ της ενσωματωμένης τεκμηρίωσης και της υλοποίησης μπορεί να οδηγήσουν σε σοβαρές παρανοήσεις σχετικά με τον τρόπο που αναμένεται να συμπεριφέρεται το σύστημα. Εξετάστε το ενδεχόμενο να διορθώσετε αυτά τα σφάλματα για να αποφύγετε τη σύγχυση για προγραμματιστές, χρήστες και ελεγκτές.
Ενημέρωση: Μερικώς διορθωμένο. Παραπλανητική τεκμηρίωση που απευθύνεται σε τραβήξτε το αίτημα # 75 και τραβήξτε το αίτημα # 77.
Η ομάδα 1 ιντσών αναφέρει:
Διορθώσαμε παραπλανητικά έγγραφα. Η συμπλήρωση των εγγράφων θα γίνει αργότερα.
[L11] Παραγγελίες DoS είναι δυνατές όταν χρησιμοποιείτε άγκιστρα
Η OrderMixin
Το συμβόλαιο υλοποιεί λειτουργικότητα για την πλήρωση γενικών εντολών ανταλλαγής εκτός αλυσίδας που θα μπορούσαν να έχουν προϋποθέσεις για την επιτυχία τους. Κατά την πλήρωση της παραγγελίας, η παραγγελία μπορεί ελέγξτε τις προκαθορισμένες συνθήκες «κατηγορήματος». πριν συνεχίσετε με την εκτέλεση.
Ωστόσο, επειδή αυτές οι βασικές συνθήκες θα μπορούσαν να στοχεύσουν τη λογική οποιουδήποτε αυθαίρετου συμβολαίου, ένας κακόβουλος κατασκευαστής θα μπορούσε να εξαπατήσει τους παραλήπτες ώστε να πιστέψουν ότι μια παραγγελία συμπεριφέρεται σωστά και ότι είναι έγκυρη όταν την ελέγχετε εκτός αλυσίδας, αλλά στη συνέχεια αποτυγχάνει όταν προσπαθεί να εκπληρώσει την ίδια παραγγελία στην αλυσίδα. Αυτή η αλλαγή στη συμπεριφορά του κατηγορήματος θα μπορούσε είτε να γίνει εκ των προτέρων σε κάποια μεταβλητή κατάσταση από την οποία εξαρτώνται τα κατηγορήματα, εξετάζοντας το αέριο που αποστέλλεται ή ακόμα και ποιες διευθύνσεις εμπλέκονται στην κλήση, είτε με κάποια άλλη λογική.
Επιπλέον, εάν ο κατασκευαστής όρισε α αλληλεπίδραση κατά τη διάρκεια της ανταλλαγής, τη interactionTarget
Το συμβόλαιο θα μπορούσε να επαναφερθεί ή να ανακαλέσει το επίδομα για να αποτρέψει την επιτυχή πλήρωση της παραγγελίας, οδηγώντας ουσιαστικά στο ίδιο αποτέλεσμα με τα κακόβουλα κατηγορήματα.
Αν και τα περιουσιακά στοιχεία δεν θα κινδυνεύουν, οι χρήστες ή τα ρομπότ που βρίσκουν μια ευνοϊκή παραγγελία θα έχουν το αυξημένο βάρος να προσπαθήσουν να εντοπίσουν αυτού του είδους τις ανεπιθύμητες παραγγελίες που μπορεί να φαίνονται νόμιμες στην επιφάνεια. Σε περίπτωση που δεν καταφέρουν να προσδιορίσουν αυτού του είδους τις παραγγελίες, θα επιβαρυνθούν με σπατάλη φυσικού αερίου. Για να μειώσετε τον αριθμό των ανεπιθύμητων παραγγελιών, εξετάστε το ενδεχόμενο να περιορίσετε τους διαθέσιμους στόχους για αυτά τα άγκιστρα. Επίσης, εξετάστε το ενδεχόμενο να προειδοποιήσετε τους χρήστες σχετικά με αυτήν τη δυνατότητα πριν επιχειρήσουν να εκπληρώσουν παραγγελίες.
Ενημέρωση: Δεν φτιάχτηκε. Η ομάδα 1 ιντσών αναφέρει:
Το χειριζόμαστε αυτό στο backend μας και θα σκεφτούμε τρόπους για να ειδοποιήσουμε πιθανούς λήπτες για το ζήτημα.
[L12] Η στρογγυλοποίηση μπορεί να είναι δυσμενής για taker
Στο OrderMixin
και OrderRFQMixin
συμβάσεις, όταν μια παραγγελία εκπληρώνεται και ο παραλήπτης παρέχει μόνο α makingAmount
or takingAmount
ποσό, το πρωτόκολλο επιχειρεί να υπολογίσει το αντίστοιχο ποσό της ανταλλαγής.
Υπάρχουν δύο ζητήματα με αυτούς τους υπολογισμούς, το πρώτο είναι ότι δεν υπάρχει τεκμηρίωση ή λογική που να περιορίζει τον αριθμό των δεκαδικών που πρέπει να χρησιμοποιούν οι παράμετροι ποσού, τα οποία εξετάσαμε στο Μη τεκμηριωμένες δεκαδικές υποθέσεις θέμα.
Το δεύτερο ζήτημα είναι ότι, κατά τη διάρκεια αυτών των υπολογισμών, το πρωτόκολλο στρογγυλοποιείται υπέρ του κατασκευαστή. Το ζήτημα της στρογγυλοποίησης μπορεί να επιδεινωθεί πολύ όταν οι σιωπηρές δεκαδικές παραδοχές παραβιάζονται, αλλά ακόμα και όταν όλα είναι στους αναμενόμενους όρους, η στρογγυλοποίηση θα συμβεί με μικρά, περίεργα ποσά.
Εξετάστε το ενδεχόμενο να επιτρέψετε στον αποδέκτη να καθορίσει ένα ελάχιστο ποσό makerAsset
περιουσιακό στοιχείο που είναι διατεθειμένοι να λάβουν μαζί με ένα μέγιστο ποσό takerAsset
περιουσιακό στοιχείο που είναι διατεθειμένοι να ανταλλάξουν, έτσι ώστε η αποδοχή οποιασδήποτε στρογγυλοποίησης είναι πιο σαφής.
Ενημέρωση: Δεν φτιάχτηκε. Η ομάδα 1 ιντσών αναφέρει:
Το ποσό κατωφλίου θα πρέπει να είναι αρκετό για την προστασία του λήπτη.
[L13] Αντιφατικός χειρισμός παραγγελιών όταν λείπουν οι παράμετροι
Στο OrderMixin
σύμβαση, το fillOrderTo
η λειτουργία πραγματοποιεί εσωτερικές κλήσεις στο _callGetMakerAmount
και _callGetTakerAmount
λειτουργεί κάθε φορά που επιχειρείται γέμισμα και είτε το makingAmount
ή η takingAmount
οι παράμετροι είναι μηδέν, αντίστοιχα, ή εάν το makingAmount
η τιμή είναι μεγαλύτερη από την remainingMakerAmount
αξία.
Η _callGetMakerAmount
και _callGetTakerAmount
οι κλήσεις θα οδηγήσουν σε αναστροφές εάν η παραγγελία δεν δημιουργήθηκε με το getMakerAmount
or getTakerAmount
παραμέτρους, αντίστοιχα, και εκτελείται μερική πλήρωση.
An ενσωματωμένο σχόλιο παράλληλα _callGetMakerAmount
και ένα ενσωματωμένο σχόλιο παράλληλα _callGetTakerAmount
ισχυριστείτε ότι "επιτρέπονται μόνο ολόκληρα γεμίσματα" εάν η παραγγελία δεν δημιουργήθηκε με getMakerAmount
or getTakerAmount
παραμέτρους.
Ωστόσο, υπάρχουν διαδρομές κώδικα για τις οποίες αυτό δεν ισχύει, επειδή αυτές οι διαδρομές δεν ελέγχουν το length
s και των δύο getMakerAmount
και getTakerAmount
παραμέτρους.
Συγκεκριμένα, Όταν ένα taker
ορίζει α takerAmount
αξία για μια παραγγελία που έχει μόνο α getMakerAmount
, εκτός αν αυτή η κλήση σε getMakerAmount
επιστρέφει ποσό μεγαλύτερο από remainingMakerAmount
, μια μερική συμπλήρωση μπορεί να εκτελεστεί σε αντίθεση με την ενσωματωμένη τεκμηρίωση.
Αυτό αφήνει ασαφή τη σκοπιμότητα αυτών των διαδρομών κώδικα. Εάν αυτή είναι η αναμενόμενη συμπεριφορά, εξετάστε το ενδεχόμενο να τροποποιήσετε την ενσωματωμένη τεκμηρίωση ώστε να είναι πιο σαφής. Εάν πρόκειται για ακούσια συμπεριφορά, σκεφτείτε πάντα να ελέγχετε τα μήκη και των δύο getMakerAmount
και την getTakerAmount
παραμέτρους ταυτόχρονα, έτσι ώστε η υλοποίηση να ενισχύει τη συμπεριφορά που περιγράφεται από την ενσωματωμένη τεκμηρίωση.
Ενημέρωση: Διορθώθηκε το τραβήξτε το αίτημα # 79.
[L14] Χρήση κλήσεων Chainlink που έχουν καταργηθεί
Η ChainlinkCalculator
Το συμβόλαιο προορίζεται να χρησιμοποιηθεί για την αναζήτηση χρησμών Chainlink. Το κάνει μέσω πραγματοποίησης κλήσεων προς τους latestTimestamp
και latestAnswer
μεθόδους, και τα δύο έχουν καταργηθεί. Στην πραγματικότητα, οι μέθοδοι δεν υπάρχουν πλέον στο API των Chainlink aggregators από την έκδοση τρία.
Για να αποφύγετε πιθανές μελλοντικές ασυμβατότητες με τους χρησμούς Chainlink, εξετάστε το ενδεχόμενο να χρησιμοποιήσετε το latestRoundData
αντ' αυτού.
Ενημέρωση: Διορθώθηκε το τραβήξτε το αίτημα # 67.
Σημειώσεις και πρόσθετες πληροφορίες
[N01] Δεν γίνεται εισαγωγή διεπαφών
Η AggregatorInterface
Η διεπαφή φαίνεται να είναι ένα υποσύνολο κώδικα που έχει αντιγραφεί από ChainLink
του δημόσιου αποθετηρίου κώδικα. Η πλήρης διεπαφή περιλαμβάνεται στο ChainLink
πακέτο npm συμβολαίου.
Όταν είναι δυνατόν, για να μειώσετε την πιθανότητα αναντιστοιχιών διεπαφών και προκύπτοντα ζητήματα, αντί να επαναπροσδιορίσετε και/ή να ξαναγράψετε τις διεπαφές ενός άλλου έργου, σκεφτείτε να χρησιμοποιήσετε διεπαφές που είναι εγκατεστημένες μέσω των επίσημων πακέτων npm τους.
Ενημέρωση: Διορθώθηκε το τραβήξτε το αίτημα # 66.
[N02] Καταργημένες εξαρτήσεις έργου
Κατά την εγκατάσταση του εξαρτήσεις του έργου, η NPM προειδοποιεί ότι ένα από τα πακέτα που έχουν εγκατασταθεί, Highlight
, "δεν θα υποστηρίζεται πλέον ούτε θα λαμβάνει ενημερώσεις ασφαλείας στο μέλλον".
Παρόλο που είναι απίθανο αυτό το πακέτο να προκαλέσει κίνδυνο ασφάλειας, εξετάστε το ενδεχόμενο αναβάθμισης της εξάρτησης που χρησιμοποιεί αυτό το πακέτο σε μια διατηρημένη έκδοση.
Ενημέρωση: Διορθώθηκε το τραβήξτε το αίτημα # 64.
[N03] Οι εξωτερικές κλήσεις για προβολή των μεθόδων δεν είναι στατικές
Σε όλο το μεγαλύτερο μέρος της βάσης κωδικών, το πρωτόκολλο πραγματοποιεί ρητά εξωτερικές κλήσεις μέσω του OpenZeppelin functionStaticCall
μέθοδος για τον περιορισμό της δυνατότητας για αλλαγές κατάστασης όταν είτε δεν είναι αναμενόμενες είτε δεν είναι επιθυμητές. Ωστόσο, στο ChainlinkCalculator
σύμβαση, παρά την πρόθεση πραγματοποίησης εξωτερικών κλήσεων μόνο προς view
μεθόδους στους χρησμούς Chainlink, τις εξωτερικές κλήσεις στο singlePrice
και doublePrice
οι λειτουργίες δεν γίνονται μέσω ρητών staticcall
s.
Αν και δεν εντοπίσαμε άμεσες ανησυχίες για την ασφάλεια που απορρέουν από αυτό, για να μειώσουμε την επιφάνεια επίθεσης, να βελτιώσουμε τη συνέπεια και να διευκρινίσουμε την πρόθεση, εξετάστε το ενδεχόμενο να χρησιμοποιήσετε ρητά staticcall
s, για όλες τις εξωτερικές κλήσεις προς view
λειτουργίες στο ChainlinkCalculator
σύμβαση.
Ενημέρωση: Δεν φτιάχτηκε. Η ομάδα 1 ιντσών αναφέρει:
Πιστεύουμε ότι η συντακτική πολυπλοκότητα ακυρώνει τις βελτιώσεις στη συνέπεια.
[N04] Μη αποτυχία νωρίς με μη έγκυρες παραγγελίες
Στο OrderMixin
σύμβαση, το fillOrderTo
η συνάρτηση χειρίζεται την ειδική κατάσταση όταν μια παραγγελία δεν έχει υποβληθεί προηγουμένως (remainingMakerAmount == 0
), αλλά δεν χειρίζεται ρητά την κατάσταση όταν η παραγγελία δεν είναι πλέον έγκυρη (remainingMakerAmount == 1
).
Στο τελευταίο σενάριο, η λειτουργία θα επανέλθει τελικά, αλλά μόνο μετά την καύση μη ασήμαντων ποσοτήτων αερίου. Για να διευκρινίσετε την πρόθεση, να αυξήσετε την αναγνωσιμότητα και να μειώσετε τη χρήση αερίου, εξετάστε το ενδεχόμενο να χειριστείτε ρητά το σενάριο μη έγκυρης παραγγελίας προς την αρχή της λειτουργίας.
Ενημέρωση: Διορθώθηκε το τραβήξτε το αίτημα # 68.
[N05] Τα συμβόλαια βοηθού δεν επισημαίνονται ως αφηρημένα
Στο Solidity, η λέξη-κλειδί abstract
χρησιμοποιείται για συμβάσεις που είτε δεν είναι λειτουργικές από μόνες τους, είτε δεν προορίζονται να χρησιμοποιηθούν ως τέτοιες. Αντι αυτου, abstract
οι συμβάσεις κληρονομούνται από άλλες συμβάσεις στο σύστημα για τη δημιουργία αυτόνομων λειτουργικών συμβάσεων.
Σε όλη τη βάση κώδικα, υπάρχουν παραδείγματα συμβάσεων βοηθού που δεν επισημαίνονται ως αφηρημένα, παρά το γεγονός ότι δεν προορίζονται να αναπτυχθούν από μόνα τους. Για παράδειγμα, το AmountCalculator
, ChainlinkCalculator
, ImmutableOwner
, NonceManager
, να PredicateHelper
όλα τα συμβόλαια αποτελούνται από ένα βασικό σύνολο λειτουργιών που προορίζονται να χρησιμοποιηθούν από κληρονομικά συμβόλαια.
Εξετάστε το ενδεχόμενο να επισημάνετε τις συμβάσεις βοηθού ως abstract
να δηλώνουν ξεκάθαρα ότι έχουν σχεδιαστεί αποκλειστικά για να προσθέτουν λειτουργικότητα σε συμβόλαια που τα κληρονομούν.
Ενημέρωση: Δεν φτιάχτηκε. Η ομάδα 1 ιντσών αναφέρει:
Αυτοί οι βοηθοί μπορούν να αναπτυχθούν χωριστά. Κληρονομούνται μόνο για εξοικονόμηση αερίου.
[N06] Ασυνεπής σειρά λειτουργιών
Σε όλη τη βάση κώδικα, η σειρά συναρτήσεων ακολουθεί γενικά το συνιστώμενη παραγγελία στον Οδηγό Solidity Style, το οποίο είναι: constructor
, fallback
, external
, public
, internal
, private
.
Ωστόσο, στο OrderMixin
σύμβαση, το public
checkPredicate
η συνάρτηση αποκλίνει από τον οδηγό στυλ, διχοτομώντας το external
λειτουργίες.
Για να βελτιώσετε τη συνολική αναγνωσιμότητα του έργου, εξετάστε το ενδεχόμενο να τυποποιήσετε τη σειρά των συναρτήσεων σε όλη τη βάση κωδικών, όπως συνιστάται από τον Οδηγό στυλ Solidity.
Ενημέρωση: Διορθώθηκε το τραβήξτε το αίτημα # 69.
[N07] Ασυνεπής ροή πλήρωσης παραγγελίας
Η OrderMixin
και RFQOrderMixin
και τα δύο συμβόλαια χειρίζονται την πλήρωση υπογεγραμμένων παραγγελιών, αλλά η γενική ροή παραγγελιών μεταξύ των δύο συμβάσεων είναι ασυνεπής.
OrderMixin
'μικρό fillOrderTo
η συνάρτηση ακολουθεί αυτή τη γενική ροή (ψευδοκώδικας):
if ((takingAmount == 0) == (makingAmount == 0))
else if (takingAmount == 0)
else (handle makingAmount == 0) THEN swapTokens
Ενώ RFQOrderMixin
είναι ανάλογο fillOrderRFQTo
η συνάρτηση ακολουθεί αυτή τη ροή (ψευδοκώδικας):
if (takingAmount == 0 && makingAmount == 0)
else if (takingAmount == 0)
else if (makingAmount == 0)
else revert THEN swapTokens
Δεν υπάρχουν πληροφορίες από την τεκμηρίωση σχετικά με το γιατί διαφέρει η πρώτη υπό όρους σε καθεμία από αυτές τις δύο συναρτήσεις ή γιατί takingAmount
και makingAmount
δεν μπορεί και τα δύο να είναι μηδέν στην τελευταία συνάρτηση. Επίσης, η περίπτωση που και οι δύο α makingAmount
και σε έναν takingAmount
παρέχονται είναι πολύ πιο εύκολο να αιτιολογηθεί στο fillOrderRFQTo
λειτουργία, αφού αντιμετωπίζεται ξεκάθαρα στην τελική else
ΟΙΚΟΔΟΜΙΚΟ ΤΕΤΡΑΓΩΝΟ.
Για να διευκρινιστεί η πρόθεση και να αυξηθεί η συνολική αναγνωσιμότητα του κώδικα, εξετάστε το ενδεχόμενο είτε να τυποποιήσετε τη γενική ροή παραγγελιών σε αυτά τα δύο συμβόλαια είτε να τεκμηριώσετε ρητά γιατί υπάρχουν οι διαφορές.
Ενημέρωση: Δεν φτιάχτηκε. Η ομάδα 1 ιντσών αναφέρει:
Αυτό οφείλεται στις προσαρμοσμένες συναρτήσεις τιμολόγησης στις οριακές παραγγελίες. Από
getMakerAmount
μπορεί δυνητικά να διαφέρει ουσιαστικά απόgetTakerAmount
, σκεφτήκαμε ότι είναι καλύτερο να μην γίνει η προεπιλεγμένη επιλογή για τον λήπτη, καθώς πιθανότατα θα τους μπερδέψει σε περιπτώσεις που αυτοί οι λήπτες θα είναι διαφορετικοί.
[N08] Τα μηνύματα σφάλματος έχουν ασυνεπή μορφή ή παραπλανητικά
Σε όλη τη βάση κώδικα, το require
και revert
Τα μηνύματα σφάλματος, τα οποία προορίζονται να ειδοποιούν τους χρήστες για τις ιδιαίτερες συνθήκες που προκαλούν την αποτυχία μιας συναλλαγής, διαπιστώθηκε ότι είχαν ασυνεπή μορφή.
Για παράδειγμα, κάθε ένα από τα μηνύματα σφάλματος στις γραμμές 85 της OrderMixin.sol
, 16 της ERC721ProxySafe.sol
, να 26 της Permitable.sol
χρησιμοποιήστε ένα διαφορετικό στυλ.
Επιπλέον, ορισμένα μηνύματα σφάλματος είναι παραπλανητικά:
Τα μηνύματα λάθους προορίζονται να ειδοποιούν τους χρήστες σχετικά με συνθήκες αποτυχίας, επομένως θα πρέπει να παρέχουν αρκετές πληροφορίες ώστε να μπορούν να γίνουν οι κατάλληλες διορθώσεις για την αλληλεπίδραση με το σύστημα. Τα μη ενημερωτικά μηνύματα λάθους βλάπτουν σημαντικά τη συνολική εμπειρία χρήστη, μειώνοντας έτσι την ποιότητα του συστήματος. Επιπλέον, τα μηνύματα λάθους με ασυνεπή μορφή μπορεί να δημιουργήσουν περιττή σύγχυση. Επομένως, εξετάστε το ενδεχόμενο να ελέγξετε ολόκληρη τη βάση κώδικα για να βεβαιωθείτε ότι κάθε require
και revert
Η δήλωση έχει ένα μήνυμα σφάλματος που είναι σταθερά μορφοποιημένο, ακριβές, ενημερωτικό και φιλικό προς το χρήστη.
Ενημέρωση: Εν μέρει σταθερό σε τραβήξτε το αίτημα # 81.
[N09] Ασυνεπής χρήση μεταβλητών ονομαστικής επιστροφής
Υπάρχει μια ασυνεπής χρήση των ονομασμένων μεταβλητών επιστροφής στο OrderMixin
σύμβαση. Μερικές λειτουργίες επιστρέφουν μεταβλητές με όνομα, οι υπολοιποι επιστρέφουν ρητές τιμές, και άλλοι να δηλώσετε μια μεταβλητή επιστροφής με όνομα αλλά να την παρακάμψετε με ρητή δήλωση επιστροφής.
Εξετάστε το ενδεχόμενο υιοθέτησης μιας συνεπούς προσέγγισης για την απόδοση τιμών σε όλη τη βάση κώδικα, αφαιρώντας όλες τις ονομασμένες μεταβλητές επιστροφής, δηλώνοντάς τις ρητά ως τοπικές μεταβλητές και προσθέτοντας τις απαραίτητες δηλώσεις επιστροφής όπου χρειάζεται. Αυτό θα βελτίωνε τόσο τη σαφήνεια όσο και την αναγνωσιμότητα του κώδικα και μπορεί επίσης να βοηθήσει στη μείωση των παλινδρομήσεων κατά τη διάρκεια μελλοντικών αναπαραγόντων κώδικα.
Ενημέρωση: Διορθώθηκε το τραβήξτε το αίτημα # 73.
[N10] Ο υπολογισμός κατακερματισμού παραγγελίας δεν είναι ανοιχτός στο API
Η external
λειτουργίες remaining
, remainingRaw
και remainingsRaw
όλοι αναμένουν κατακερματισμό παραγγελίας για επιτυχή λειτουργία.
Ωστόσο, η βοηθητική λειτουργία _hash
, που επιστρέφει τον κατακερματισμό μιας παραγγελίας, έχει private
ορατότητα. Αυτό σημαίνει ότι οι χρήστες θα πρέπει να συσκευάσουν μέρη των παραγγελιών και τις συμβολοσειρές τομέα με μη αυτόματο τρόπο, προκειμένου να αποκτήσουν τον κατακερματισμό μιας παραγγελίας.
Για να αποφύγετε την πιθανότητα σφαλμάτων κατά τον υπολογισμό των κατακερματισμών παραγγελιών και για να παρέχετε στους χρήστες μια μέθοδο για τη δημιουργία του αντίστοιχου κατακερματισμού μιας παραγγελίας, εξετάστε το ενδεχόμενο να επεκτείνετε την ορατότητα του _hash
λειτουργία για public
και αναπαράσταση του ονόματος σε hash
να είναι συνεπής με τον υπόλοιπο κώδικα.
Ενημέρωση: Διορθώθηκε το τραβήξτε το αίτημα # 74.
[Ν11] Σημασιολογική υπερφόρτωση της χαρτογράφησης
Η _remaining
χαρτογράφηση στο OrderMixin
Το συμβόλαιο είναι σημασιολογικά υπερφορτωμένο για την παρακολούθηση της κατάστασης των παραγγελιών και του υπόλοιπου ποσού των διαθέσιμων στοιχείων για αυτές τις παραγγελίες.
Οι τρεις καταστάσεις που μπορεί να αναλάβει είναι:
0
: Ο κατακερματισμός παραγγελίας δεν έχει εμφανιστεί ακόμα.1
: Η παραγγελία είτε έχει ακυρωθεί είτε έχει ολοκληρωθεί πλήρως.- Κάθε
uint
μεγαλύτερο από1
: Το υπόλοιποmakerAmount
διαθέσιμο για συμπλήρωση με την παραγγελία συν1
.
Αυτή η σημασιολογική υπερφόρτωση απαιτεί τύλιγμα και ξετύλιγμα αυτής της τιμής κατά τη διάρκεια lookup
, cancellation
, initialization
, να storage
.
Η σημασιολογική υπερφόρτωση και η απαραίτητη λογική για την ενεργοποίησή της μπορεί να είναι επιρρεπής σε σφάλματα και μπορεί να καταστήσει τη βάση κώδικα πιο δυσνόητη και δυσκολότερη, μπορεί επίσης να ανοίξει την πόρτα για παλινδρομήσεις σε μελλοντικές ενημερώσεις του κώδικα.
Για να βελτιώσετε την αναγνωσιμότητα του κώδικα, εξετάστε το ενδεχόμενο να παρακολουθήσετε την κατάσταση ολοκλήρωσης των παραγγελιών σε ξεχωριστή αντιστοίχιση.
Ενημέρωση: Δεν φτιάχτηκε. Η ομάδα της 1 ίντσας ανέφερε ότι μια επιδιόρθωση θα αυξήσει το κόστος φυσικού αερίου για το fillOrder
λειτουργία.
[N12] Οι παραγγελίες με άδεια επιτρέπουν κλήσεις σε αυθαίρετες συμβάσεις
Η OrderMixin
συμβόλαιο κληρονομεί το Permitable
σύμβαση που επιτρέπει την πλήρωση εντολών μίας συναλλαγής με περιουσιακά στοιχεία που αποδέχονται τέτοια permit
εκκλήσεις για τροποποίηση των δικαιωμάτων.
Ωστόσο, η καλεί στο Permitable
σύμβαση μην επικυρώσετε εάν ο στόχος είναι ένα επιτρεπόμενο περιουσιακό στοιχείο, ούτε εάν πρόκειται για περιουσιακό στοιχείο, το οποίο θα μπορούσε να επιτρέψει σε έναν κακόβουλο χρήστη να περάσει τη διεύθυνση μιας αυθαίρετης σύμβασης που θα μπορούσε να εκτελέσει άλλη κλήση πριν ολοκληρωθεί η πλήρωση της παραγγελίας.
Αν και η σύμβαση είναι προστατεύονται από την επανεισδοχήΣυνιστάται πάντα η μείωση της επιφάνειας επίθεσης και η αποτροπή της κλήσης εξωτερικών συμβάσεων κατά την εκτέλεση. Εξετάστε το ενδεχόμενο να περιορίσετε το περιουσιακό στοιχείο που εμπλέκεται στην άδεια στα στοιχεία που εμπλέκονται στην παραγγελία ή σε μια λευκή λίστα στοιχείων για το πρωτόκολλο.
Ενημέρωση: Δεν φτιάχτηκε. Η ομάδα 1 ιντσών αναφέρει:
OrderMixin
δεν έχει στην πραγματικότητα πληροφορίες για τα πραγματικά διακριτικά καθώςmakerAsset
καιtakerAsset
Μερικές φορές υπάρχουν διακομιστές μεσολάβησης ή άλλα ενδιάμεσα συμβόλαια και οι πληροφορίες σχετικά με τα πραγματικά διακριτικά αποθηκεύονται σε ορισμένα αυθαίρετα byte. Επομένως, δεν υπάρχει βιώσιμος τρόπος να περιοριστεί ποιο περιουσιακό στοιχείοpermit
καλείται.
[N13] solhint
δεν ενεργοποιείται ποτέ ξανά
Σε όλη τη βάση κωδικών, υπάρχουν μερικά solhint-disable
δηλώσεις, ειδικά αυτές που βρίσκονται στο Διαδίκτυο 23 και on line 41 of RevertReasonParser.sol
, που δεν τερματίζονται με solhint-enable
.
Υπέρ της σαφήνειας και να είμαστε όσο το δυνατόν πιο περιοριστικοί κατά την απενεργοποίηση solhint
, σκεφτείτε να χρησιμοποιήσετε solhint-disable-line
or solhint-disable-next-line
αντί αυτού, παρόμοια με τη γραμμή 16 του ίδιου αρχείου.
Ενημέρωση: Διορθώθηκε το τραβήξτε το αίτημα # 72.
[N14] Τυπογραφικά λάθη
Η βάση κώδικα περιέχει τα ακόλουθα τυπογραφικά λάθη:
Επιπλέον του έργου README
(εκτός πεδίου εφαρμογής για αυτόν τον έλεγχο) περιέχει τα ακόλουθα τυπογραφικά λάθη:
Εξετάστε το ενδεχόμενο να διορθώσετε αυτά τα τυπογραφικά λάθη για να βελτιώσετε την αναγνωσιμότητα του κώδικα.
Ενημέρωση: Διορθώθηκε το τραβήξτε το αίτημα # 71 και τραβήξτε το αίτημα # 77.
[N15] Χρήση του uint
αντί του uint256
Για να ευνοηθεί η σαφήνεια, όλες οι περιπτώσεις του uint
θα πρέπει να δηλωθεί ως uint256
. Ειδικότερα, όσοι βρίσκονται στο for
βρόχους σε γραμμές 98 και 119 of OrderMixin.sol
και γραμμές 16 και 30 of PredicateHelper.sol
.
Ενημέρωση: Διορθώθηκε το τραβήξτε το αίτημα # 70.
συμπεράσματα
Βρέθηκαν 3 θέματα υψηλής σοβαρότητας. Ορισμένες αλλαγές προτάθηκαν για να ακολουθηθούν οι βέλτιστες πρακτικές και να μειωθεί η πιθανή επιφάνεια επίθεσης.
- &
- 7
- Σχετικά
- πρόσβαση
- Σύμφωνα με
- Λογαριασμός
- απέναντι
- Πράξη
- ενεργειών
- Πρόσθετος
- διεύθυνση
- Πλεονέκτημα
- Όλα
- Επιτρέποντας
- ήδη
- Αν και
- Ποσά
- ανάλυση
- api
- πλησιάζω
- επιχειρήματα
- γύρω
- προσόν
- Ενεργητικό
- έλεγχος
- Πίσω μέρος
- Αρχή
- είναι
- ΚΑΛΎΤΕΡΟΣ
- βέλτιστες πρακτικές
- Κομμάτι
- bots
- χτίζω
- κλήση
- ο οποίος
- περιπτώσεις
- Αιτία
- ΚΡΙΚΟΣ ΑΛΥΣΙΔΑΣ
- αλλαγή
- έλεγχος
- έλεγχοι
- κωδικός
- συγκρότημα
- κατάσταση
- σύγχυση
- δόμηση
- Περιέχει
- σύμβαση
- συμβάσεις
- Διορθώσεις
- Δικαστικά έξοδα
- θα μπορούσε να
- Ζευγάρι
- δημιουργός
- Νόμισμα
- Ρεύμα
- ημερομηνία
- συμφωνία
- Denial of Service
- ανάπτυξη
- Υπηρεσίες
- προγραμματιστές
- Ανάπτυξη
- DID
- διαφέρω
- διαφορετικές
- τομέα
- διπλασιαστεί
- δυναμικός
- Νωρίς
- άκρη
- ενθαρρύνει
- ειδικά
- ETH
- Συμβάν
- εκδηλώσεις
- πάντα
- παράδειγμα
- ανταλλαγή
- αναμένεται
- εμπειρία
- Εκμεταλλεύομαι
- Χαρακτηριστικά
- Πεδία
- Τελικά
- Όνομα
- σταθερός
- Ευελιξία
- ροή
- ακολουθήστε
- Βρέθηκαν
- πλήρη
- λειτουργία
- χρήματα
- μελλοντικός
- παιχνίδι
- GAS
- General
- Δίνοντας
- εξαιρετική
- καθοδηγήσει
- Χειρισμός
- χασίσι
- κατακερματισμός
- που έχει
- βοήθεια
- Ψηλά
- υψηλά
- Πως
- HTTPS
- Υβριδικό
- προσδιορίσει
- Επίπτωση
- εφαρμογή
- σημαντικό
- εισαγωγή
- περιλαμβάνονται
- Συμπεριλαμβανομένου
- Αυξάνουν
- αυξημένη
- πληροφορίες
- πληροφορίες
- Υποδομή
- ιδέες
- πρόθεση
- τόκος
- περιβάλλον λειτουργίας
- συμμετέχουν
- θέματα
- IT
- large
- μεγαλύτερος
- οδηγήσει
- που οδηγεί
- Βιβλιοθήκη
- Περιωρισμένος
- γραμμή
- Ρευστότητα
- Εισηγμένες
- Λίστες
- τοπικός
- κοίταξε
- αναζήτηση
- μεγάλες
- κατασκευαστής
- Κατασκευή
- αγορά
- Μέμπουλ
- καθρέπτης
- μοντέλο
- πλέον
- Δημοφιλέστερα
- και συγκεκριμένα
- Νέες δυνατότητες
- μη-ανταλλάξιμα
- μη-ανταλλάξιμα μάρκες
- επίσημος ανώτερος υπάλληλος
- ανοίξτε
- λειτουργίες
- Επιλογή
- μαντείο
- τάξη
- παραγγελιών
- ΑΛΛΑ
- ιδιοκτήτης
- πρότυπο
- Δημοφιλής
- παρόν
- πρόληψη
- τιμή
- τιμολόγηση
- ιδιωτικός
- διαδικασια μας
- σχέδιο
- έργα
- προστασία
- πρωτόκολλο
- παρέχουν
- παρέχει
- πληρεξούσιο
- δημόσιο
- δημοσιεύει
- αγορά
- ποιότητα
- αύξηση
- Πραγματικότητα
- μείωση
- εξάρτηση
- αναφέρουν
- Εκθέσεις
- Αποθήκη
- απαιτήσεις
- ΠΕΡΙΦΕΡΕΙΑ
- Αποτελέσματα
- Επιστροφές
- ανασκόπηση
- Κίνδυνος
- γύρους
- τρέξιμο
- SDK
- ασφάλεια
- Υπηρεσίες
- σειρά
- Shared
- Μερίδια
- αλλαγή
- παρόμοιες
- Απλούς
- small
- έξυπνος
- Έξυπνα συμβόλαια
- So
- στερεότητα
- το spam
- ειδικά
- Δαπάνες
- Spot
- Εκκίνηση
- Κατάσταση
- Δήλωση
- Μελών
- Κατάσταση
- χώρος στο δίσκο
- στυλ
- υποβάλλονται
- επιτυχία
- επιτυχής
- Επιτυχώς
- υποστήριξη
- υποστηριζόνται!
- Επιφάνεια
- διακόπτης
- σύστημα
- στόχος
- δοκιμή
- Δοκιμές
- δοκιμές
- Μέσω
- παντού
- ώρα
- μαζι
- ένδειξη
- κουπόνια
- τροχιά
- Παρακολούθηση
- συναλλαγή
- Εμπιστευθείτε
- μοναδικός
- ενημερώσεις
- us
- χρηστικότητα
- USD
- USDT
- Χρήστες
- χρησιμότητα
- αξία
- Δες
- ορατότητα
- περιμένετε
- Τι
- whitelist
- εντός
- χωρίς
- Εργασία
- αξία
- μηδέν