Detsember 6, 2021
Sissejuhatus
. Tally meeskond palus meil üle vaadata ja auditeerida lepingute komplekt, mille lõppeesmärk on parandada ühiseid juhtimislepinguid ja anda neile rohkem paindlikkust. Vaatasime koodi ja avaldame nüüd oma tulemused.
Süsteemi ülevaade
Süsteem tugineb kolmele põhilepingule:
- A
SafeGuard
lepingu mall. See leping onadmin
kohtaTimelock
leping ja lisab mooduli rollide kihiTimelock
tegevused. See leping määratleb mitu rolli, millel on eraldi vastutus ja juurdepääs pakkumise olekule (järjekorda seadmine, tühistamine ja täitmine). - A
SafeGuardFactory
leping rakendab uutSafeGuard
ja vastav uusTimelock
leping. Seejärel määrab see ajaluku aadressiSafeGuard
lepingu ja registreerib uueSafeGuard
sisseRegistry
. - A
Registry
mis sisaldab kasutusele võetud nimekirjaSafeGuards
nende vastavatega versiooni number.
. SafeGuardFactory
põhimõtteliselt kudema a uus SafeGuard
millal iganes seda kutsutakse. A SafeGuard
on ümberringi Timelock
toimingud mis lisab iga toimingu jaoks erinevad ja eraldatud rollid. Rollid on üles ehitatud OpenZeppelini kasutamise kaudu AccessControlEnumerable
leping, mis annab rohkem paindlikkust ja ühilduvust multisig-rahakottidele ja ärikasutusjuhtudele, kus mitmel aadressil võib olla sama jagatud roll.
Värskenda: aasta PR # 10, Tally meeskond on otsustanud eemaldada Registry
leping. Kasutatud kaitsemeetmete loend on nüüd salvestatud SafeGuardFactory
leping, aastal safeGuards
loendatav komplekt koos nende versiooniga, mis on talletatud safeGuardVersion
kaardistamine.
rollid
. SafeGuard
Leping määratleb järgmised rollid:
Värskenda: . CREATOR_ROLE
roll eemaldati probleemi L02 lahendusena.
Ulatus
Auditeerisime kohustust b2c63a9dfc4090be13320d999e7c6c1d842625d3
Euroopa safeguard
hoidla. Reguleerimisalasse kuuluvad nutikad lepingud contracts
kataloog. Siiski, mocks
kataloog loeti reguleerimisalast väljas olevaks.
Eeldused
Süsteem ei ole mõeldud uuendamiseks. The Registry
aadress on määratud konstruktoris Euroopa SafeGuardFactory
ja igaüks SafeGuard
saab Timelock
määrata ainult üks kord. See tähendab, et kui uus Registry
kasutusele võetakse uus SafeGuardFactory
tuleb ka kasutusele võtta. Kui Timelock
teostusmuudatused, vana SafeGuard
s vananevad ja tuleb kasutusele võtta uued.
Lisaks sõltub süsteem suuresti a rakendamisest Timelock
leping mida ei peetud selle auditi jaoks kohaldatavaks. Meeskond ei ole selle rakendamist veel lõpetanud Timelock
leping. Selle aruande kirjutamise ajal Ühendi ajaluku rakendamine on projektis kasutusel.
Koodibaasi on ühe nädala jooksul auditeerinud kaks audiitorit ja siin tutvustame oma järeldusi.
Kriitiline raskusaste
Puudub.
Kõrge raskusastmega
[H01] ETH saab lukustada Timelock
leping
. Tally
meeskond põhines algselt nende rakendamisel GovernorBravoDelegate
Liitleping.
Selle auditi käigus on Tally
meeskond avastas piirang Ühenduse kuberneris, kus ETH saadeti otse Timelock
ei ole juhtimisettepanekute jaoks kasutamiseks saadaval ja kuigi see pole püsivalt kinni jäänud, nõuab selle hankimiseks keerukat lahendust.
Selle põhjuseks on asjaolu, et kuberneri rakendamine nõuab kogu ettepaneku väärtuse lisamist msg.value
konto, mis käivitab täitmise, mitte mingil viisil kasutama Timelock
ETH fondid.
. sama probleem tuvastati hiljem SafeGuard
täitmine ja meeskond on probleemist teadlik ja seda parandatakse.
Probleemi lahendamisel kaaluge selle lähenemisviisi kasutamist vastu võetud OpenZeppelini raamatukogu sama numbri jaoks.
Värskenda: Fikseeritud kohustus 7337db227edda83533be586135d96ddac4f5bf29
.
[H02] SafeGuardFactory saab külmutada
. Registry
lepingu eesmärk on jälgida kõiki SafeGuards
et SafeGuardFactory
toodab. Sellel on väline register
funktsioon mida selleks otstarbeks kasutatakse.
Samal ajal SafeGuardFactory
on oma konstrueerijas kohaliku määramine registry
väärtus sisendväärtuseks. Selle väärtust pole võimalik muuta registry
muutuv ja sel põhjusel, kui uus Registry
kasutusele, tuleb kasutusele võtta ka uus tehas.
. SafeGuardFactory
on createSafeGuard
funktsioon, vastutab esimese eest uue juurutamine SafeGuard
, Siis Uus Timelock
aadressiga SafeGuard
as admin
, seejärel seadistades timelock
muutuja SafeGuard
leping ja lõpuks registreerides SafeGuard
registris.
Probleem on selles, et iga kõne createSafeGuard
võib sundida ebaõnnestuma ründaja, kes saab otse registreerida uue deterministliku aadressi SafeGuard
enne selle loomist. Alati kui leping loob a new
Näiteks, suurendatakse selle nonce'i ja lepingu uue eksemplari juurutamise aadressi saab määrata algse lepingu aadressi ja selle nonce'i järgi. Seetõttu saab ründaja eelnevalt välja arvutada paljud aadressid, kus uus SafeGuards
kasutusele võetakse ja need aadressid registreeritakse Registry
helistades register
funktsiooni. Selle tulemuseks on kõned createSafeGuard
et naasma alates Registry
sisaldab juba aadressi.
Vältimaks väliste osalejate avalikku helistamist Register
lepinguga, kaaluge juurdepääsu piiramist register
funktsioon kõnede vastuvõtmiseks ainult poolt SafeGuardFactory
.
Värskenda: Kinnitatud PR # 10. Tally meeskond on eemaldanud Registry
leping.
Keskmise raskusastmega
Puudub.
Madal raskusaste
[L01] Kommenteeritud kood
. Registry
leping sisaldab kommenteeritud koodirida. Loetavuse parandamiseks kaaluge selle koodibaasist eemaldamist.
Värskenda: Kinnitatud PR # 10 ja pühenduma 7fd27df16fc879d990d36a167a0b6e719e578558
.
[L02] SafeGuardi administraator võib määrata looja rolli mis tahes aadressile
. SafeGuard
leping määratleb a rolli CREATOR_ROLE
mis, nagu nimigi ütleb, on määratud kaitsemehhanismi loojale.
Siiski kutsudes the,en grantRole
funktsioon AccessControlEnumerable
leping OpenZeppelini lepinguteegis saab administraator anda selle rolli mis tahes aadressile. See võib tekitada segadust, kuna looja SafeGuard
saab olla ainult SafeGuardFactory
.
Kogu koodibaasi jooksul on seda rolli kasutatud ainult selleks, et piirata kasutajatel suhtlemist setTimelock
funktsioon Euroopa SafeGuard
leping. Süsteem tagab selle disaini järgi setTimelock
funktsioon helistada saab ainult üks kordAlates jooksul SafeGuardFactory
leping.
Kaaluge eemaldamist CREATOR_ROLE
rolli alates SafeGuard
lepingu ja kasutades onlyOwner
muutus aasta setTimelock
funktsiooni.
Värskenda: Kinnitatud PR # 10.
[L03] Vale liidese määratlus ja rakendamine
. ISafeGuard
liides ei määratle queueTransactionWithDescription
funktsioon aastal rakendatud SafeGuard
leping ja samal ajal määratleb see __abdicate, __queueSetTimelockPendingAdmin ja __executeSetTimelockPendingAdmin funktsioone, kuid neid ei rakendata.
Koodibaasi korrektsuse ja järjepidevuse parandamiseks kaaluge koodi ümberkujundamist ISafeGuard
liides, et see vastaks täpselt SafeGuard
rakendamine.
Värskenda: Fikseeritud kohustus 7fd27df16fc879d990d36a167a0b6e719e578558
.
[L04] Dokumendistringid puuduvad
Mõnel koodibaasi lepingul ja funktsioonil puudub dokumentatsioon. Näiteks, mõned funktsioonid aasta SafeGuard
leping.
Lisaks kasutavad mõned dokstringid mitteametlikku keelt, näiteks üks üle setTimelock
toimib SafeGuard
leping.
See takistab ülevaatajatel aru saada koodi kavatsusest, mis on oluline mitte ainult turvalisuse, vaid ka õigsuse õigeks hindamiseks. Lisaks parandavad dokumendistringid loetavust ja hõlbustavad hooldust. Nad peaksid selgesõnaliselt selgitama funktsioonide eesmärki või kavatsusi, stsenaariume, mille korral need võivad ebaõnnestuda, rolle, mida neile on lubatud kutsuda, tagastatud väärtusi ja väljastatud sündmusi.
Kaaluge kõigi funktsioonide (ja nende parameetrite) põhjalikku dokumenteerimist, mis on osa lepingute avalikust API-st. Tundlikku funktsiooni rakendavad funktsioonid, isegi kui need pole avalikud, tuleks samuti selgelt dokumenteerida. Dokumendistringide kirjutamisel kaaluge järgimist Ethereumi looduslik spetsifikatsioonivorming (NatSpec).
Värskenda: Osaliselt fikseeritud PR # 10. Kogu koodibaasi erinevatele funktsioonidele on lisatud korralikud dokumendistringid. Lisaks praegustele muudatustele kaaluge siiski järgmiste muudatuste tegemist.
- lisama
description
kui@param
ülaltoodud dokstringisqueueTransactionWithDescription
funktsioon - lisama
@param
aasta dokstring ülecreateSafeGuard
funktsiooni sisseSafeGuardFactory
leping - lisama
@return
docstringides funktsioonide kohalSafeGuardFactory
leping.
[L05] Kasutu või korduv kood
Koodibaasis on kohti, kus koodi kas korratakse või pole vaja. Mõned näited on järgmised:
- Read 29–32 Euroopa
Registry
leping on kasutud, sest_add
funktsioonEnumerableSet
leping juba teostab neid kontrolle juba seatud väärtuste vastu. - Liinid 62, 67, 73 ja 78 Euroopa
SafeGuard
kõik lepingud kordavad täpselt sama toimingut. Kaaluge selle kapseldamist sisemisse funktsiooni, et vältida koodi dubleerimist. - Read 62–63 ja 67-68 of
SafeGuard
korratakse. Kaaluge nende kapseldamist üheks sisemiseks funktsiooniks. - Kasutamine
gasleft
et määrata, kui palju gaasi tuleb funktsiooni kutses edastadaexecuteTransaction
on tarbetu. Selle põhjuseks on asjaolu, et sellel täitmise hetkel kasutatakse kogu järelejäänud gaasi täitmise jätkamiseks. Kui see pole selgesõnaline, kaaluge eemaldamistgas
parameeter kõnest.
Kaaluge soovitatud paranduste rakendamist puhtama koodi loomiseks ning koodibaasi järjepidevuse ja modulaarsuse parandamiseks.
Värskenda: Kinnitatud PR # 10 ja pühenduma 7fd27df16fc879d990d36a167a0b6e719e578558
.
Märkused ja lisateave
[N01] Ebaühtlane stiil
Koodibaasis on kohti, kus stiilierinevused mõjutavad loetavust, muutes koodi mõistmise keerulisemaks. Mõned näited on järgmised:
- .
Registry
Leping kasutab kogu lepingus dokumentide jaoks erinevaid stiile. - .
SafeGuard
leping kiirgab sündmust, kuiqueueTransactionWithDescription
kutsutakse, kuid teistes tehingutega tegelevates funktsioonides sündmusi ei väljastata. - aasta
SafeGuard
leping, mõnikord väärtus kasutatakse nimelise parameetrina ja mõnikord _value kasutatakse.
Võttes arvesse väärtust, mida järjepidev kodeerimisstiil projekti loetavusele lisab, kaaluge standardse kodeerimisstiili jõustamist linteritööriistade (nt Solhint) abil.
Värskenda: Kinnitatud PR # 10 ja pühenduma 7fd27df16fc879d990d36a167a0b6e719e578558
.
[N02] Litsents puudub
Järgmistel koodibaasi lepingutel puudub SPDX litsentsi identifikaator.
Kompilaatori hoiatuste vaigistamiseks ja koodibaasi järjepidevuse suurendamiseks kaaluge litsentsi identifikaatori lisamist. Seda tehes kaaluge viitamist spdx.dev suunised.
Värskenda: Kinnitatud PR # 10 ja pühenduma 7fd27df16fc879d990d36a167a0b6e719e578558
.
[N03] OpenZeppelini lepingu sõltuvus ei ole kinnitatud
Et vältida ootamatut käitumist juhul, kui tulevastes värskendustes avaldatakse purunevaid muudatusi OpenZeppelini lepingute teek, kaaluge selle sõltuvuse versiooni kinnitamist jaotisesse pakett.json faili.
Värskenda: Kinnitatud PR # 10.
[N04] Solidity kompilaatori versioon pole kinnitatud
Kaaluge kogu koodibaasi versiooni kinnitamist Solidity kompilaator selle uusimale stabiilsele versioonile. See peaks aitama vältida ootamatute vigade sissetoomist ühildumatute tulevaste väljaannete tõttu. Konkreetse versiooni valimiseks peaksid arendajad arvestama nii projekti jaoks vajalike kompilaatori funktsioonidega kui ka teadaolevate vigade loend seostatakse iga Solidity kompilaatori versiooniga.
Värskenda: Kinnitatud PR # 10.
[N05] Kirjaviga
Erinevatel juhtudel kogu koodibaasis sõna role
on valesti kirjutatud kui rol
. Üks selline näide on siin dokstring sees constructor
Euroopa SafeGuard
leping.
Koodi loetavuse parandamiseks kaaluge nende kirjavigade parandamist.
Värskenda: Osaliselt fikseeritud PR # 10. Kuigi õigekiri role
on parandatud, peaks kommentaar "määratletud administraatori aadressi administraatori roll" olema "määratletud administraatori aadressi määramine administraatori rolliks". Lisaks on tekstis "käivita" valesti kirjutatud SafeGuard
leping peale rida 69, rida 82, rida 96 ja rida 110 ja "saadaval" on valesti kirjutatud rida 70, rida 83, rida 97, rida 111. Kaaluge ka mitteametlike sõnade asendamist, näiteks "hakkab" in SafeGuard
leping ametlike alternatiividega, nagu „lähen“.
[N06] Deklareerige uint kui uint256
Koodibaasis on mitmeid juhtumeid, kus muutujad deklareeritakse uint
asemel andmetüüp uint256
. Näiteks eta
muutuja QueueTransactionWithDescription
sündmus Euroopa SafeGuard
leping.
Selgesõnalisuse soodustamiseks kõik juhtumid uint
tuleks deklareerida kui uint256
.
Värskenda: Kinnitatud PR # 10 ja pühenduma 7fd27df16fc879d990d36a167a0b6e719e578558
.
[N07] Kasutamata import
. SafeGuard
leping impordib console
lepingu, kuid ei kasuta seda kunagi.
Koodi loetavuse parandamiseks kaaluge kasutamata impordi eemaldamist.
Värskenda: Kinnitatud PR # 10.
Järeldused
Leiti üks suur ja mitu muud väiksemat turvaauku ning soovitati soovitusi ja parandusi.
- &
- juurdepääs
- konto
- üle
- tegevus
- meetmete
- Täiendavad lisad
- aadress
- admin
- Materjal: BPA ja flataatide vaba plastik
- juba
- Kuigi
- API
- lähenemine
- ümber
- audit
- Põhimõtteliselt
- on
- vead
- äri
- helistama
- juhtudel
- Põhjus
- muutma
- tasu
- kood
- Kodeerimine
- ühine
- SEGU
- segadus
- tasu
- sisaldab
- jätkama
- leping
- lepingud
- võiks
- looja
- Praegune
- andmed
- tegelema
- Disain
- Arendajad
- erinev
- avastasin
- Töötage välja
- ETH
- sündmus
- sündmused
- näide
- tehas
- FUNKTSIOONID
- Lõpuks
- esimene
- Määrama
- Paindlikkus
- avastatud
- funktsioon
- raha
- tulevik
- GAS
- andmine
- eesmärk
- valitsemistava
- Kuberner
- suunised
- võttes
- aitama
- siin
- Suur
- Kuidas
- HTTPS
- rakendatud
- Suurendama
- kasvanud
- Interface
- IT
- teatud
- keel
- hiljemalt
- Raamatukogu
- litsents
- joon
- nimekiri
- kohalik
- lukus
- Vaatasin
- Tegemine
- Vastama
- modulaarne
- multisign
- Muu
- esitada
- protsess
- projekt
- ettepanek
- avalik
- avaldama
- registreerima
- Pressiteated
- aru
- Hoidla
- Tulemused
- läbi
- turvalisus
- komplekt
- kehtestamine
- jagatud
- nutikas
- Tarkvaralepingud
- kindlus
- riik
- stiil
- süsteem
- Läbi
- läbi kogu
- aeg
- töövahendid
- jälgida
- Tehingud
- Uudised
- us
- Kasutajad
- väärtus
- Haavatavused
- Rahakotid
- nädal
- WHO
- jooksul
- sõnad
- kirjutamine