Joulukuu 6, 2021
esittely
- pistelasku tiimi pyysi meitä tarkistamaan ja tarkastamaan joukon sopimuksia, joiden lopullisena tavoitteena on parantaa yhteisiä hallintosopimuksia ja antaa niille enemmän joustavuutta. Tarkastelimme koodia ja julkaisemme nyt tulokset.
Järjestelmäkatsaus
Järjestelmä perustuu kolmeen pääsopimukseen:
- A
SafeGuard
sopimusmalli. Tämä sopimus onadmin
aTimelock
sopimus ja lisää kerroksen modulaarisia roolejaTimelock
n toimia. Tässä sopimuksessa määritellään useita rooleja, joilla on erillinen vastuu ja käyttöoikeus ehdotuksen tilaan (jono, peruutus ja toteutus). - A
SafeGuardFactory
sopimus ottaa käyttöön uudenSafeGuard
ja vastaava uusiTimelock
sopimus. Sitten se asettaa aikalukon osoitteen sisälläSafeGuard
sopimuksen ja rekisteröi uudenSafeGuard
osaksiRegistry
. - A
Registry
joka sisältää luettelon käyttöönotetuistaSafeGuards
niiden vastaavien kanssa versionumero.
- SafeGuardFactory
pohjimmiltaan kutee a uusi SafeGuard
aina kun sitä kutsutaan. A SafeGuard
on kietoa Timelock
toiminta joka lisää erilaisia ja erillisiä rooleja jokaiselle toiminnolle. Roolit on strukturoitu OpenZeppelinin avulla AccessControlEnumerable
sopimus antaa lisää joustavuutta ja yhteensopivuutta multisig-lompakoihin ja yrityskäyttötapauksiin, joissa useilla osoitteilla voi olla sama jaettu rooli.
Päivitys: In PR # 10, Tally-tiimi on päättänyt poistaa Registry
sopimus. Luettelo käyttöönotetuista suojatoimista on nyt tallennettu SafeGuardFactory
sopimuksessa safeGuards
enumerable joukko sekä niiden versio, joka on tallennettu safeGuardVersion
kartoitus.
Roolit
- SafeGuard
Sopimus määrittelee seuraavat roolit:
Päivitys: - CREATOR_ROLE
rooli on poistettu ongelman L02 korjauksena.
Laajuus
Tarkastimme sitoumuksen b2c63a9dfc4090be13320d999e7c6c1d842625d3
että safeguard
arkisto. Soveltamisalaan kuuluvat älykkäät sopimukset contracts
hakemistosta. Kuitenkin mocks
hakemisto katsottiin soveltamisalan ulkopuolelle.
Oletukset
Järjestelmää ei ole tarkoitettu päivitettäväksi. The Registry
osoite on asetettu rakentajassa että SafeGuardFactory
ja jokainen SafeGuard
voi olla Timelock
asetettu vain kerran. Tämä tarkoittaa, että jos uusi Registry
otetaan käyttöön uusi SafeGuardFactory
on myös otettava käyttöön. Jos Timelock
täytäntöönpanon muutokset, vanha SafeGuard
s vanhentuvat, ja uusia on otettava käyttöön.
Lisäksi järjestelmä on vahvasti riippuvainen a Timelock
sopimus joka katsottiin tämän tarkastuksen ulkopuolelle. Tiimi ei ole vielä saanut päätökseen ohjelman täytäntöönpanoa Timelock
sopimus. Tätä raporttia kirjoitettaessa yhdisteen aikalukon toteutus on käytössä projektissa.
Koodikannan on tarkastanut kaksi tilintarkastajaa viikon aikana ja esittelemme tässä havainnot.
Kriittinen vakavuus
Ei mitään.
Erittäin vakava
[H01] ETH voidaan lukita sisään Timelock
sopimus
- Tally
tiimi perustui alun perin toteutuksiinsa GovernorBravoDelegate
Yhdistelmäsopimus.
Tämän tarkastuksen aikana Tally
tiimi löysi rajoitus Yhdisteen kuvernöörissä, jossa ETH lähetetään suoraan Timelock
ei ole hallintoehdotusten käytettävissä, ja vaikka se ei ole pysyvästi jumissa, se vaatii monimutkaisen kiertotavan hakemisen.
Tämä johtuu siitä, että kuvernöörin toteutus edellyttää, että ehdotuksen koko arvo liitetään msg.value
suorittamisen käynnistävällä tilillä, eikä käytä millään tavalla Timelock
ETH-rahastot.
- sama ongelma tunnistettiin myöhemmin SafeGuard
täytäntöönpano ja tiimi on tietoinen ongelmasta ja on parhaillaan korjaamassa sitä.
Kun korjaat ongelmaa, harkitse lähestymistavan käyttöä OpenZeppelin-kirjaston hyväksymä samalle numerolle.
Päivitys: Kiinteä sitoutuminen 7337db227edda83533be586135d96ddac4f5bf29
.
[H02] SafeGuardFactory voidaan jäädyttää
- Registry
sopimuksen tarkoituksena on pitää kirjaa kaikista SafeGuards
että SafeGuardFactory
tuottaa. Siinä on ulkoinen register
toiminto jota käytetään tähän tarkoitukseen.
Samaan aikaan, SafeGuardFactory
on rakentajassaan paikallisen määritys registry
arvo syötearvoon. Ei ole mahdollisuutta muuttaa arvoa registry
muuttuja ja tästä syystä, jos uusi Registry
otetaan käyttöön, myös uusi tehdas on otettava käyttöön.
- SafeGuardFactory
on createSafeGuard
toiminto, joka vastaa ensin ottaa käyttöön uutta SafeGuard
, sitten uusi Timelock
osoitteen kanssa SafeGuard
as admin
, asettamalla sitten timelock
muuttuja SafeGuard
sopimus ja lopuksi rekisteröimällä SafeGuard
rekisterissä.
Ongelmana on, että kaikki puhelut createSafeGuard
voi pakottaa epäonnistumaan hyökkääjällä, joka voi rekisteröidä suoraan uuden deterministisen osoitteen SafeGuard
ennen sen luomista. Aina kun sopimus luo new
esimerkki, sen nonce kasvaa, ja osoite, jossa sopimus uusi ilmentymä otettaisiin käyttöön, voidaan määrittää alkuperäisen sopimusosoitteen ja sen nonce-osoitteen perusteella. Siksi hyökkääjä voi laskea etukäteen monet osoitteet, joissa uusi SafeGuards
otetaan käyttöön ja rekisteröi nämä osoitteet Registry
soittamalla register
toiminto. Tämä johtaisi puheluihin createSafeGuard
että palautua koska Registry
sisältää jo osoitteen.
Välttääksesi ulkopuolisten toimijoiden soittavan julkisesti Register
sopimusta, harkitse pääsyn rajoittamista register
toiminto vastaanottaa puhelut yksinomaan SafeGuardFactory
.
Päivitys: Kiinteä sisään PR # 10. Tally-tiimi on poistanut Registry
sopimus.
Keskivaikea
Ei mitään.
Matala vakavuus
[L01] Kommentoitu koodi
- Registry
sopimus sisältää kommentoitu koodirivi. Luettavuuden parantamiseksi harkitse sen poistamista koodikannasta.
Päivitys: Kiinteä sisään PR # 10 ja sitoutua 7fd27df16fc879d990d36a167a0b6e719e578558
.
[L02] SafeGuardin järjestelmänvalvoja voi määrittää luojan roolin mille tahansa osoitteelle
- SafeGuard
sopimus määrittelee roolin a CREATOR_ROLE
joka, kuten nimestä voi päätellä, on määrätty suojan luojalle.
Kuitenkin kutsumalla Ishayoiden opettaman grantRole
toiminto AccessControlEnumerable
sopimus OpenZeppelin-sopimuskirjastossa järjestelmänvalvoja voi myöntää tämän roolin mihin tahansa osoitteeseen. Tämä voi aiheuttaa hämmennystä, koska luoja SafeGuard
voi olla vain SafeGuardFactory
.
Koko koodikannan ajan tätä roolia on käytetty vain rajoittamaan käyttäjiä olemasta vuorovaikutuksessa koodikannan kanssa setTimelock
toiminto että SafeGuard
sopimus. Suunnittelultaan järjestelmä varmistaa sen setTimelock
toiminto voidaan soittaa vain kerranalkaen puitteissa SafeGuardFactory
sopimus.
Harkitse poistamista CREATOR_ROLE
rooli SafeGuard
sopimusta ja käyttämällä onlyOwner
muutos vuonna setTimelock
toiminto.
Päivitys: Kiinteä sisään PR # 10.
[L03] Väärä rajapinnan määritelmä ja toteutus
- ISafeGuard
käyttöliittymä ei määrittele queueTransactionWithDescription
toiminto toteutettu vuonna SafeGuard
sopimus, ja samalla se määrittelee __abdicate, __queueSetTimelockPendingAdmin ja __executeSetTimelockPendingAdmin toimintoja, mutta niitä ei ole toteutettu.
Parantaaksesi koodikannan oikeellisuutta ja johdonmukaisuutta, harkitse koodin muokkaamista uudelleen ISafeGuard
käyttöliittymä vastaa täsmälleen SafeGuard
täytäntöönpanoa.
Päivitys: Kiinteä sitoutuminen 7fd27df16fc879d990d36a167a0b6e719e578558
.
[L04] Puuttuvat dokumentit
Joistakin koodikannan sopimuksista ja toiminnoista puuttuu dokumentaatio. Esimerkiksi, joitakin toimintoja vuonna SafeGuard
sopimus.
Lisäksi jotkin asiakirjat käyttävät epävirallista kieltä, kuten yksi Edellä setTimelock
toiminto SafeGuard
sopimus.
Tämä estää arvioijia ymmärtämästä koodin tarkoitusta, joka on olennaista arvioitaessa oikein paitsi turvallisuutta myös oikeellisuutta. Lisäksi dokumenttisarjat parantavat luettavuutta ja helpottavat ylläpitoa. Niiden tulee selkeästi selittää toimintojen tarkoitus tai tarkoitus, skenaariot, joissa ne voivat epäonnistua, roolit, joilla niitä voidaan kutsua, palautetut arvot ja lähetetyt tapahtumat.
Harkitse kaikkien sopimusten julkiseen sovellusliittymään kuuluvien toimintojen (ja niiden parametrien) perusteellista dokumentointia. Arkaluonteisia toimintoja toteuttavat toiminnot, vaikka ne eivät olisi julkisia, tulee myös dokumentoida selkeästi. Kun kirjoitat asiakirjoja, harkitse seuraavaa Ethereumin luonnollinen erittelymuoto (NatSpec).
Päivitys: Osittain kiinteä PR # 10. Asianmukaiset dokumentit on lisätty useisiin toimintoihin koko koodikannassa. Harkitse kuitenkin nykyisten muutosten lisäksi seuraavien muutosten tekemistä:
- Lisää
description
kuten@param
yllä olevassa asiakirjassaqueueTransactionWithDescription
toiminto - Lisää
@param
vuonna opetus EdelläcreateSafeGuard
toimintoSafeGuardFactory
sopimus - Lisää
@return
dokumenttimerkkijonoissa funktioiden yläpuolellaSafeGuardFactory
sopimus.
[L05] Hyödytön tai toistuva koodi
Koodikannassa on paikkoja, joissa koodia joko toistetaan tai sitä ei tarvita. Joitakin esimerkkejä ovat:
- Rivit 29-32 että
Registry
sopimus ovat hyödyttömiä, koska_add
toimintoEnumerableSet
sopimus suorittaa jo nämä tarkastukset jo asetettuja arvoja vastaan. - Rivit 62, 67, 73 ja 78 että
SafeGuard
sopimus toistaa täsmälleen samaa toimintoa. Harkitse sen kapseloimista sisäiseen toimintoon koodin päällekkäisyyden välttämiseksi. - Rivit 62-63 ja 67-68 of
SafeGuard
toistetaan. Harkitse niiden kapseloimista yhdeksi sisäiseksi toiminnoksi. - Käyttö
gasleft
määrittääksesi kuinka paljon kaasua tulee välittää funktion kutsussaexecuteTransaction
on tarpeeton. Tämä johtuu siitä, että siinä suorituspisteessä koko jäljellä oleva kaasu käytetään suorittamisen jatkamiseen. Jos tämä ei ole selkeyden vuoksi, harkitse sen poistamistagas
parametri puhelusta.
Harkitse ehdotetun korjauksen käyttämistä puhtaamman koodin tuottamiseksi ja koodikannan johdonmukaisuuden ja modulaarisuuden parantamiseksi.
Päivitys: Kiinteä sisään PR # 10 ja sitoutua 7fd27df16fc879d990d36a167a0b6e719e578558
.
Huomautuksia ja lisätietoja
[N01] Epäjohdonmukainen tyyli
Koodipohjassa on paikkoja, joissa tyylierot vaikuttavat luettavuuteen, mikä vaikeuttaa koodin ymmärtämistä. Joitakin esimerkkejä ovat:
- -
Registry
sopimus käyttää eri tyylejä dokumenttijonoille koko sopimuksessa. - -
SafeGuard
sopimus lähettää tapahtuman, kunqueueTransactionWithDescription
kutsutaan, mutta tapahtumia ei lähetetä muissa tapahtumia käsittelevissä toiminnoissa. - In
SafeGuard
sopimus joskus arvo käytetään nimettynä parametrina ja joskus _arvo käytetään.
Ottaen huomioon arvon, jonka johdonmukainen koodaustyyli lisää projektin luettavuuteen, harkitse standardin koodaustyylin pakottamista linterityökalujen, kuten Solhintin, avulla.
Päivitys: Kiinteä sisään PR # 10 ja sitoutua 7fd27df16fc879d990d36a167a0b6e719e578558
.
[N02] Ajokortti puuttuu
Seuraavista koodikannan sopimuksista puuttuu SPDX-lisenssitunniste.
Harkitse lisenssitunnisteen lisäämistä kääntäjien varoitusten hiljentämiseksi ja koodikannan johdonmukaisuuden lisäämiseksi. Harkitse sitä tehdessään viittaamista spdx.dev ohjeita.
Päivitys: Kiinteä sisään PR # 10 ja sitoutua 7fd27df16fc879d990d36a167a0b6e719e578558
.
[N03] OpenZeppelin-sopimuksen riippuvuutta ei ole kiinnitetty
Odottamattoman käyttäytymisen estämiseksi, jos rikkovia muutoksia julkaistaan tulevissa OpenZeppelin Contracts -kirjasto, harkitse tämän riippuvuuden version kiinnittämistä package.json tiedosto.
Päivitys: Kiinteä sisään PR # 10.
[N04] Solidity-kääntäjäversiota ei ole kiinnitetty
Harkitse koko koodipohjan version kiinnittämistä Solidity-kääntäjä sen uusimpaan vakaaseen versioon. Tämän pitäisi auttaa estämään odottamattomien virheiden syntyminen yhteensopimattomien tulevien julkaisujen vuoksi. Tietyn version valinnassa kehittäjien tulee ottaa huomioon sekä projektin tarvitsemat kääntäjän ominaisuudet että luettelo tunnetuista bugeista jokaiseen Solidity-kääntäjäversioon.
Päivitys: Kiinteä sisään PR # 10.
[N05] Kirjoitusvirhe
Useissa tapauksissa koko koodikannassa sana role
on kirjoitettu väärin nimellä rol
. Yksi tällainen esimerkki löytyy sisällä oleva dokumenttimerkkijono constructor
että SafeGuard
sopimus.
Harkitse näiden kirjoitusvirheiden korjaamista koodin luettavuuden parantamiseksi.
Päivitys: Osittain kiinteä PR # 10. Vaikka oikeinkirjoitus role
on korjattu, kommentin "aseta järjestelmänvalvojan rooli määritettyyn järjestelmänvalvojan osoitteeseen" tulisi olla "aseta järjestelmänvalvojan rooli määritettyyn järjestelmänvalvojan osoitteeseen". Lisäksi "suorita" on kirjoitettu väärin SafeGuard
sopimus päällä line 69, line 82, line 96 ja line 110 ja "käytettävissä" on kirjoitettu väärin line 70, line 83, line 97, line 111. Harkitse myös epävirallisten sanojen, kuten esim "aikoo" in SafeGuard
sopimus muodollisten vaihtoehtojen kanssa, kuten "menee".
[N06] Ilmoita uint nimellä uint256
Koodikannassa on useita esiintymiä, joissa muuttujat ilmoitetaan uint
tietotyypin sijaan uint256
. Esimerkiksi eta
muuttuja QueueTransactionWithDescription
tapahtumaa varten että SafeGuard
sopimus.
Yksinkertaisuuden suosimiseksi, kaikki tapaukset uint
tulee ilmoittaa uint256
.
Päivitys: Kiinteä sisään PR # 10 ja sitoutua 7fd27df16fc879d990d36a167a0b6e719e578558
.
[N07] Käyttämätön tuonti
- SafeGuard
sopimus tuonti console
sopimusta, mutta ei koskaan käytä sitä.
Voit parantaa koodin luettavuutta poistamalla käyttämättömät tuonnit.
Päivitys: Kiinteä sisään PR # 10.
Päätelmät
Yksi korkea ja useita muita pieniä haavoittuvuuksia on löydetty ja suosituksia ja korjauksia on ehdotettu.
- &
- pääsy
- Tili
- poikki
- Toiminta
- toimet
- lisä-
- osoite
- admin
- Kaikki
- jo
- Vaikka
- api
- lähestymistapa
- noin
- tilintarkastus
- Pohjimmiltaan
- ovat
- Bugs
- liiketoiminta
- soittaa
- tapauksissa
- Aiheuttaa
- muuttaa
- lataus
- koodi
- Koodaus
- Yhteinen
- Yhdiste
- sekaannus
- harkinta
- sisältää
- jatkaa
- sopimus
- sopimukset
- voisi
- luoja
- Nykyinen
- tiedot
- tekemisissä
- Malli
- kehittäjille
- eri
- löysi
- Kehittää
- ETH
- tapahtuma
- Tapahtumat
- esimerkki
- tehdas
- Ominaisuudet
- Vihdoin
- Etunimi
- Korjata
- Joustavuus
- löytyi
- toiminto
- varat
- tulevaisuutta
- GAS
- Antaminen
- tavoite
- hallinto
- Maaherra
- suuntaviivat
- ottaa
- auttaa
- tätä
- Korkea
- Miten
- HTTPS
- täytäntöön
- Kasvaa
- kasvoi
- liitäntä
- IT
- tunnettu
- Kieli
- uusin
- Kirjasto
- Lisenssi
- linja
- Lista
- paikallinen
- lukittu
- Katsoin
- Tekeminen
- ottelu
- modulaarinen
- Multisig
- Muut
- esittää
- prosessi
- projekti
- ehdotus
- julkinen
- julkaista
- ilmoittautua
- Tiedotteet
- raportti
- säilytyspaikka
- tulokset
- arviot
- turvallisuus
- setti
- asetus
- yhteinen
- fiksu
- Smart-sopimukset
- kiinteys
- Osavaltio
- tyyli
- järjestelmä
- Kautta
- kauttaaltaan
- aika
- työkalut
- raita
- Liiketoimet
- Päivitykset
- us
- Käyttäjät
- arvo
- haavoittuvuuksia
- Lompakot
- viikko
- KUKA
- sisällä
- sanoja
- kirjoittaminen