Joulukuu 16, 2021
esittely
- 1inch tiimi pyysi meitä tarkistamaan ja tarkastamaan ne Limit Order Protocol v2 älykkäät sopimukset. Tarkastelimme koodia ja julkaisemme nyt tulokset.
Laajuus
Tarkastimme sitoumuksen 4d94eea25e4dac6271bfd703096a5c4a4d899b4a
että 1inch/limit-order-protocol
arkisto. Sopimukseen kuuluivat seuraavat sopimukset:
- 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
Kaikki muut projektitiedostot ja hakemistot (mukaan lukien testit) sekä ulkoiset riippuvuudet ja projektit, peliteoria ja kannustinsuunnittelu jätettiin myös tämän tarkastuksen ulkopuolelle. Ulkoisten koodi- ja sopimusriippuvuuksien oletettiin toimivan dokumentoidulla tavalla, ja 1 tuuman tarjoamien taustapalvelujen oletettiin toimivan protokollan parhaan edun mukaisesti.
Yleinen terveys
Yleisesti ottaen pidimme projektin koodikannan olevan luettavissa ja hyvin organisoitu, vaikka se voisi hyötyä laajemmasta dokumentaatiosta, erityisesti kokoonpanokoodilohkoista, protokollan reunatapauksista, käytettävistä resursseista/predikaateista/ulkoisista resursseista, tarjottavan taustapalvelun vastuut/rajoitukset ja toimijoiden välinen vuorovaikutus. Hankkeessa tehdään paljon töitä tehdäkseen toimista kaasutehokkaita, joskus jopa riskillä, että koodista tulee vaikeampi järkeillä; käsittelemme alla tähän liittyviä kysymyksiä. Koko tarkastuksen ajan 1 tuuman tiimi oli erittäin tavoitettavissa, reagoiva ja erittäin helppo työskennellä.
Järjestelmän yleiskatsaus
Limit Order Protocol mahdollistaa tilauksen makers
allekirjoittaa tilauksia ketjun ulkopuolelta tokeninvaihtoa varten. Protokolla helpottaa sitten aiemmin allekirjoitettujen tilausten täyttämistä tilauksella takers
. Tilaukset ovat erittäin laajennettavissa, ja ne voivat staattiselta kutsua ulkoisia sopimuksia useissa kohdissa tilauksen täyttöprosessin aikana. Tämä laajennettavuus lisää protokollan hyödyllisyyttä, mutta se lisää sekä monimutkaisuutta että suuremman hyökkäyspinnan itse tilauksille.
On tärkeää huomata, että tilaustietoja ei ole ketjussa. Tilausten täyttötilaa tai peruutustilaa seurataan vain tilaushajautusten avulla. Tämä edellyttää, että tilaukset jaetaan peer-to-peer tai keskitetyn osapuolen kautta. Tässä tapauksessa 1 tuuman tiimi aikoo toimia keskitettynä osapuolena, joka kokoaa allekirjoitetut toimeksiannot ja käyttää niitä likviditeettilähteenä muille protokollilleen. Tilaukset julkaistaan oman API:n kautta, jotta käyttäjät voivat olla vuorovaikutuksessa niiden kanssa.
Tämä keskitys antaa 1 tuuman tiimille äärimmäisen hallinnan siihen, mitkä tilaukset julkaistaan ja lopulta toteutetaan. Tämä antaa heille myös mahdollisuuden sensuroida tilauksia, mikä voi olla hyödyllistä haitallisten tai harhaanjohtavien tilausten tapauksessa, mutta sitä voidaan myös käyttää väärin ja antaa heille mahdollisuuden ohjata muita käyttäjiä, jos he saavat suotuisan tilauksen, koska ne eivät näytä sitä API:n kautta.
Etuoikeutetut roolit
Vaikka sopimukset, joissa roolia käytetään, eivät kuuluneet soveltamisalaan, yksi etuoikeutettu rooli tunnistettiin. An immutableOwner
on asetettu valtakirjasopimuksen tekijälle rakennushetkellä, ja sitä käytetään rajoittamaan pääsyä valtakirjaan external
toiminnot.
Ulkoiset riippuvuudet ja luottamusoletukset
Tämän protokollan suunnittelu edellyttää ketjun ulkopuolisia ja ketjun sisäisiä komponentteja, ja tätä hybridimallia voidaan käyttää lieventämään joitain raportissamme tunnistamiamme hyökkäysvektoreita, mutta tämän ominaisuuden hinta on lisääntynyt riippuvuus 1 tuuman tiimistä ja infrastruktuurista.
Lisäksi Limit Order Protocol tarjoaa toimintoja, jotka on tarkoitettu hintojen hakemiseen Chainlink-oraakkeleista. Oletimme näiden oraakkelien olevan rehellisiä, helposti saatavilla ja toimivia.
Lisäksi tilauksen joustavuuden vuoksi on olemassa useita yhteyspisteitä ulkoisten sopimusten kanssa, joita ei ole validoitu. Tämä tarkoittaa, että pahantahtoinen käyttäjä voi väärinkäyttää tällaisia kutsuja ja esiintyä haitallisilla sopimuksilla predikaatteilla, omaisuuksilla tai oraakkeleilla suorittaakseen toimia tilausten täyttöjen aikana. Vaikka projekti on joillakin alueilla suojattu sisäänpääsyltä, tällaiset vektorit voivat aiheuttaa palvelunestohyökkäyksiä tai havaitsemattomia roskapostitilauksia. 1 tuuman tiimi on tietoinen siitä, että tiettyjä ongelmia saattaa ilmetä, kun protokollaan käytetään tuntemattomia sopimuksia, ja on ilmaissut aikomuksensa, että projekti tukee täysin vain merkittäviä "blue chip" -omaisuuksia. On kuitenkin huomattava, että jopa suosituimmissa omaisuuksissa on jokaisessa omaisuudessa luontaisia käyttäytymismalleja, jotka voivat aiheuttaa ongelmia protokollissa, jotka eivät käsittele niitä oikein, kuten maksu USDT:n siirroista tai virhekoodin palauttaminen menestys boolean cTokenien kanssa.
Tulokset
Tässä esittelemme havainnot.
Kriittinen vakavuus
Ei mitään.
Erittäin vakava
[H01] Epäjohdonmukaiset tiedot siirretty _makeCall
In OrderMixin
sopimus _makeCall
-toimintoa käytetään varojen siirtämiseen ottajalta valmistajalle ja sitten valmistajalta ottajalle. Jälkimmäisessä siirrossa _makeCall
toiminto on välitetty väärin tilauksen makerAsset
viimeisenä parametrina, kun sen pitäisi olla tilauksen makerAssetData
.
Tämän seurauksena kaikki välityspalvelimen toiminnot, jotka perustuvat makerAssetData
argumentti katkeaa.
Ollakseen johdonmukainen aiemman kutsun kanssa _makeCall
ja tue täysimääräisesti välityspalvelintoimintoja, harkitse tiedoston päivittämistä order.makerAsset
parametri order.makerAssetData
.
Päivitys: Kiinteä sisään vetopyyntö # 57.
[H02] Osittain täytetyt yksityistilaukset voivat täyttää kuka tahansa
Pöytäkirja mahdollistaa yksityisten ja julkisten tilausten luomisen. Yksityistilauksesta vain allowedSender
osoite, jonka valmistaja on ilmoittanut tilauksen luomisen yhteydessä, pystyy täyttämään tilauksen.
Kuitenkin OrderMixin
sopimus, vahvistusta varten allowedSender
osoite on väärin rajattu, mikä tarkoittaa, että se arvioidaan vain tilauksen ensimmäisen täyttöä käsittelevän logiikan sisällä. Jos yksityinen tilaus on osittain täytetty, tarkista allowedSender
osoite ei ole enää tavoitettavissa ja tilauksesta tulee kenen tahansa täytettävä.
Selvittääksesi, pitäisikö kenenkään käyttäjän pystyä täyttämään osittain täytettyjä yksityisiä tilauksia vai ei, harkitse joko nykyisen toiminnan syyn dokumentointia tai allowedSender
osoite ensimmäisen täytön soveltamisalan ulkopuolella varmistaaksesi, että se tarkistetaan aina, kun täyttöä yritetään.
Päivitys: Kiinteä sisään vetopyyntö # 58.
[H03] Haitallinen valmistaja voi hyödyntää osittaisia täyttöjä varastaakseen ottajan omaisuuden
Tilaukset osoitteesta OrderMixin
sopimus voidaan täyttää osittain. Osittaisten täyttöjen tukemiseksi protokolla vaatii tavan laskea vaihtojen molemmat puolet. Molemmat getMakerAmount
ja getTakerAmount
Tilauksen tekijä määrittelee kentät juuri tätä tarkoitusta varten.
Tilausta täytettäessä vastaanottajien on toimitettava joko makingAmount
tai takingAmount
arvot sekä a thresholdAmount
arvo. On kaksi erilaista koodipolkua, jotka voidaan valita sen perusteella, onko makingAmount
tai takingAmount
tarjottiin.
Ensimmäinen on, kun makingAmount
parametri on määritelty. Se voisi katkaista Ishayoiden opettaman makingAmount
arvo ja myös laskea takingAmount
arvo sille. Tässä tilanteessa, thresholdAmount
varmistaa, että takingAmount
otettu arvo on ei yllättävän suuri.
Toinen on, kun takingAmount
parametri on määritelty. Siinä tapauksessa tulee laskea makingAmount
arvoa, mahdollisuudella katkaisemalla sitä ja laskemalla uudelleen takingAmount
arvo, jos niin tapahtuu. Tässä tilanteessa, thresholdAmount
arvo varmistaa, että makingAmount
palautettu arvo on ei yllättävän pieni.
On olemassa kaksi hyväksikäyttömenetelmää, joista jokainen on ainutlaatuinen jollekin edellä mainituista koodipoluista. Nämä hyväksikäyttömenetelmät vaativat haitallisia getMakerAmount
ja getTakerAmount
toimintoja. Näiden toimintojen yksinkertainen toteutus käyttäytyisi samalla tavalla kuin AmountCalculator
n getMakerAmount
ja getTakerAmount
toimintoja, mutta kovakoodatulla kytkimellä, joka pakottaa ne tarvittaessa palauttamaan hyökkääjän ohjaaman arvon.
Ensimmäinen, vähemmän vakava hyväksikäyttömalli sisältää ensimmäisen koodipolun, jossa makingAmount
arvo on määritetty täyttöjärjestyksessä. Haitallinen valmistaja odottaisi täyttömääräystä, joka määrittää makingAmount
ilmestyä muistioon sen eteen. Ne tyhjentävät kaiken arvon paitsi 1 valmistajan puolelta ja sitten pakottaisivat _callGetTakerAmount
palauttaaksesi käyttäjän ilmoittaman summan thresholdAmount
arvo (tai niiden lisäys, jos se on pienempi). Kun käyttäjän tapahtuma vihdoin menee läpi, hän vaihtaa täyden thresholdAmount
arvoinen takerAsset
yhdelle yksikölle makerAsset
. Tämä hyödyntäminen on rajoitettu määrällä, jonka on antanut thresholdAmount
arvo tai määrä takerAsset
käyttäjälle sallittu LimitOrderProtocol
sopimus.
Toinen, vakavampi hyväksikäyttömalli sisältää toisen koodipolun, jossa takingAmount
arvo on määritetty. Haitallinen valmistaja odottaisi vastaavasti täyttömääräystä, jossa määritetään a takingAmount
arvo näkyy muistiossa. He ajaisivat kaupan eteen ja pakottaisivat makingAmount
palauttama arvo _callGetMakerAmount
funktio on suurempi kuin molemmat remainingMakerAmount
ja thresholdAmount
. He asettaisivat myös takingAmount
palautti arvon _callGetTakerAmount
olla määrä takerAsset
omaisuus sallittu LimitOrderProtocol
ottajan toimesta. Kun vastaanottajan kauppa menee läpi, se menee läpi katkaista makingAmount
arvo ja sitten laskea uudelleen takingAmount
arvo. Tämä uudelleenlaskenta ei kuitenkaan ole taatusti pienempi, ja tässä tapauksessa se tyhjentää saajalta kaiken takerAsset
jonka he sallivat sopimuksessa. Tässä koodipolussa thresholdAmount
arvo on varmistamalla, että makingAmount
ei ole liian matala, joten ottaa kaikki ottajat takerAsset
omaisuutta ei ole valittu. Menetettyjen varojen määrää rajoittaa takerAsset
resurssi, jonka käyttäjä on sallinut LimitOrderProtocol
sopimus.
Nämä hyväksikäytöt eivät ole mahdollisia ilman osittaisia tilauksia ja erityisesti osittaisia tilauksia haitallisilla getMakerAmount
ja getTakerAmount
toteutuksia.
Pääkysymys thresholdAmount
arvon tarkistus on, että se kattaa vain swapin toisen puolen, mutta toista puolta voidaan käsitellä frontrunningin kautta. Ei ole takeita siitä, että vastaanottajan alun perin ehdottama arvo pysyy muuttumattomana. Harkitse poistamista makingAmount
katkaisu molemmista koodipoluista ja palautus, jos tilaus ei voi tukea niin suurta täyttöä kuin pyydetään. Tekemällä tämän thresholdAmount
voidaan käyttää rajoittamaan riittävästi swapin toista puolta ja välttämään odottamatonta käyttäytymistä, jopa haitallisissa tilauksissa.
Päivitys: Kiinteä sisään vetopyyntö # 83.
Keskivaikea
[M01] Staattiset argumentit välitetään dynaamisten argumenttien jälkeen
In OrderMixin
sopimus getTakerAmount
ja getMakerAmount
bytes kenttiä käytetään argumentteina _callGetTakerAmount
ja _callGetMakerAmount
toimintoja. Nämä puhelut tarjoavat tavan laskea swapin toinen puoli toisen puolen perusteella, ja niiden avulla käyttäjät voivat täyttää tilauksia osittain.
- getTakerAmount
/getMakerAmount
kentät ovat dynaamisia muuttujia ja ne on pakattu takerAmount
ja makerAmount
arvot _callGetTakerAmount
ja _callGetMakerAmount
toimintoja. Haitallinen valmistaja voi tarjota odotettua enemmän tietoja getTakerAmount
jagetMakerAmount
kentät työntämään takerAmount
ja makerAmount
tavua ohi sen paikan, jossa niiden oletetaan olevan, kun ne dekoodataan seuraavassa funktiossa. Tämän ansiosta valmistaja voi siirtää syötettyä vastaanottaja- tai valmistajamäärää täydellä tavulla oikealle ja jopa korvata ne kokonaan, jos ylimääräiset 32 tavua dataa tarjotaan.
Käyttäjien on jo tarkistettava manuaalisesti getTakerAmount
ja getMakerAmount
kentät järjestyksessä, mutta tätä tekniikkaa on melko vaikea havaita. On myös syytä huomata, että tämä hyökkäys koskee jopa sisäisesti luotettuja getMakerAmount
ja getTakerAmount
toimintoja. Useimmille hyökkäyksille kohtuullisen kynnyssumman asettaminen estää varojen menetyksen.
Tämän estämiseksi harkitse staattisten argumenttien koodaamista ennen dynaamisia argumentteja, jotta vältytään antamasta dynaamisille argumenteille menetelmää staattisten argumenttien hallitsemiseksi.
Päivitys: Ei korjattu. 1 tuuman joukkue totesi:
Olemme erityisen varovaisia getters-validoinnin kanssa. Yritämme ottaa käyttöön sdk:ssämme getters-tarkistuksen, joka auttaa suodattamaan mahdollisesti haitallisia tilauksia.
[M02] ERC721-tilauksia voidaan muokata
On mahdollista vaihtaa muutakin kuin pelkkä ERC20:n kautta OrderMixin
ottamalla käyttöön sopimus, jolla on sama toimintovalitsin kuin IERC20:ssä transferFrom
, ja toimittamalla kyseisen sopimuksen nimellä makerAsset
tai takerAsset
järjestyksessä.
Soveltamisalan ulkopuoliset välityspalvelimet, nimittäin ERC721Proxy
, ERC721ProxySafe
ja ERC1155Proxy
sopimukset noudattavat tätä mallia tukeakseen ERC721
ja ERC1155
rahakkeita. Koska välityspalvelimia on kutsuttava samalla mallilla kuin IERC20 transferFrom
kutsu, allekirjoituksen on alettava address from
, address to
ja uint256 amount
. Kaikki muu, mitä välityspalvelimet vaativat, voidaan välittää sen jälkeen, ja se määritellään järjestyksessä makerAssetData
ja takerAssetData
.
ERC1155:t voivat luonnollisesti siirtää useita samoja tunnusmerkkejä kerralla, mikä tarkoittaa ERC1155Proxy
sopimus käyttää hyväksi amount
ala. Toisaalta, ERC721
Sillä ei ole ilmeistä käyttöä amount
ala. Koska ne edustavat ei-korjattavia tunnuksia, tietyllä tokenId:llä on vain yksi olemassa, mikä tekee amount
kenttä hyödytön. Tämän vuoksi toteutus molemmille ERC721Proxy
ja ERC721ProxySafe
sopimuksissa käytetään vaadittua amount
kenttä kuin tokenId
sen sijaan.
Tämä ylikuormitus amount
parametri luo mahdollisuuden osittaiseen täyttöön ERC721
tilauksia ostaaksesi erikseen listattuja rahakkeita alennettuun hintaan. Voi esimerkiksi olla tapaus, jossa yhdellä käyttäjällä on useita ERC721
s saman sopimuksen salliman siirtää ERC721Proxy
sopimusta ja luettelee ne erillisissä rajatoimeksiannoissa.
Jos rajatoimeksiannot tarjoavat myös getMakerAmount
ja getTakerAmount
kentät on mahdollista täyttää osittain ERC721
tilauksia. Tilauksesta lähtien amount
kenttä vastaa itse asiassa tokenId
, pahantahtoinen käyttäjä voi täyttää osittaisen täytön ERC721
korkeammalla tokenId:llä, jolloin tuloksena on a makingAmount
/takingAmount
ja ERC721
joka voisi vastata alempaa tokenId
. Tuloksena on ERC721
alaosan kanssa tokenId
siirrettäisiin hintaan (higher tokenId price) * (lower tokenId's id) / (higher tokenId's id)
.
Tällä hyväksikäytöllä on muutamia vaatimuksia:
- moninkertainen
ERC721
s samasta sopimuksesta sallitaan jompikumpiERC721
yhden omistajan valtakirja. - Avaa tilaus yhdelle
ERC721
s se ei ole alhaisintokenId
sallituista. - Osittaiset täytöt sallittu tilauksessa.
Voit poistaa kokonaan mahdollisuuden osittaiseen ERC721
täyttää, harkitse erottelemista amount
ja tokenId
argumentteja. Riippumatta siitä, erotetaanko argumentit vai ei, harkitse myös tämän dokumentointia varoittaaksesi käyttäjiä tästä käyttäytymisestä ja välttääksesi tämän kaavan tulevaisuudessa.
Päivitys: Kiinteä sisään vetopyyntö # 59.
[M03] Dokumentoimattomat desimaalioletukset
- LimitOrderProtocol
sopimus perii ChainlinkCalculator
sopimus kautta OrderMixin
sopimus. Tämä sopimus paljastaa kaksi toimintoa, jotka mahdollistavat Chainlink-oraakkelien käytön predikaattien tarkistus ja haku valmistajan määrä/vastaanottajan määrä.
Sopimuksessa tehdään kuitenkin dokumentoimattomia oletuksia desimaalien määrästä, jolla Chainlink-oraakkelit tulee raportoida, sekä desimaalien määrästä, jotka funktioparametrien tulee sisältää. Tietyissä skenaarioissa tämä voi johtaa odottamattomiin toimiin, mukaan lukien omaisuuden väärä hinnoittelu ja tahaton varojen menetys.
Tarkemmin sanottuna koko sopimuksen implisiittinen oletus on, että Chainlink-oraakkelit raportoivat 18 desimaalin tarkkuudella. Ei kuitenkaan kaikki Chainlink-oraakkelit raportoida tällä desimaalimäärällä. Itse asiassa, jos oraakkeli ilmoittaa valuuttaparin (esimerkiksi USD:n), sen tarkkuus on vain 8 desimaaleja. Koska ei ole rajoituksia joka oraakkeleja voidaan käyttää, implisiittisiä oletuksia ei pidä tehdä desimaalien määrästä, jolla ne raportoidaan.
Tähän liittyen on olemassa epäsuora oletus, että amount
parametri ChainlinkCalculator
funktiot käyttävät 18 desimaaleja yhdessä harhaanjohtavan nimenomaisen ilmoituksen kanssa, että singlePrice
toiminto Calculates price of token relative to ETH scaled by 1e18
. Todellisuudessa jopa oraakkelin kanssa ei raportti 18 desimaalilla, palautusarvo singlePrice
funktio skaalataan desimaalien lukumäärän mukaan amount
parametri, joka ei välttämättä ole 18 desimaaleja.
Vastaavasti doublePrice
funktio olettaa, että kaksi Chainlink-oraakkelia raportoivat samalla desimaalimäärällä, jolloin funktion tulos poikkeaa odotuksista.
Harkitse oletusarvojen dokumentointia, jotka koskevat desimaalien määrää, joita parametrien ja palautusarvojen tulee olla. Lisäksi harkitse joko rajoittavia laskelmia, jotka riippuvat oraakkeleista, jotka rikkovat nämä oletukset, tai ota asiaankuuluvat laskelmat huomioon todellisen desimaalien lukumäärän.
Päivitys: Kiinteä sisään vetopyyntö # 75.
Matala vakavuus
[L01] Vakioita ei ole nimenomaisesti ilmoitettu
Koodikannassa käytetään muutamia kirjaimellisia arvoja, joilla on selittämätön merkitys. Esimerkiksi:
- In
OrderMixin
sopimus_remaining
kartoitus on semanttisesti ylikuormitettu (kuten numerossa selitetään Kartoituksen semanttinen ylikuormitus) seurataksesi jäljellä olevan omaisuuden määrää osittain täytetylle tilaukselle sekä jos tilaus on täytetty kokonaan. Erityisesti,0
tarkoittaa, että tilaukseen liittyviä täyttöjä ei ole tehty,1
tarkoittaa, että tilausta ei voida enää täyttää, eikä mitään suurempaa kuin1
tarkoittaa, että tilaukseen liittyy jäljellä oleva summa, joka voidaan mahdollisesti täyttää. - In
ChainlinkCalculator
sopimus, kirjaimellinen arvo1e18
käytetäänsinglePrice
toiminto.
Parantaaksesi koodin luettavuutta ja helpottaaksesi uudelleenmuodostusta, harkitse vakion määrittämistä jokaiselle maagiselle numerolle ja anna sille selkeä ja itsestään selvä nimi. Jos arvo on monimutkainen, harkitse upotetun kommentin lisäämistä, jossa selitetään, kuinka ne laskettiin tai miksi ne on valittu.
Päivitys: Kiinteä sisään vetopyyntö # 75 ja vetopyyntö # 76.
[L02] Haitalliset osapuolet voivat estää sallittujen määräysten toteuttamisen
- OrderMixin
sopimuksen avulla valmistajan käyttäjät voivat lähettää sallittuja tilauksia joten ne voidaan suorittaa yhdessä tapahtumassa sen sijaan, että tarvittaisiin erillinen tapahtuma hyväksyntää varten. Myös tilausten vastaanottajat voivat esittämään oman lupansa tilauksen täyttämisen aikana samaan tarkoitukseen.
Kuitenkin, koska valmistajan lupa on sisällä tilata, sekä valmistajan että vastaanottajan luvat olisivat käytettävissä tilauksen täyttötapahtuman ollessa muistissa. Tämä mahdollistaisi sen, että kaikki pahantahtoiset käyttäjät voivat ottaa kyseiset luvat ja toteuttaa ne vastaavissa omaisuussopimuksissa täyttötapahtuman edellä. Koska näillä luvilla on a nonce
kaksinkertaisen kulutushyökkäyksen estämiseksi tilauksen täyttötapahtuma epäonnistuisi, koska yritetään käyttää samaa lupaa, jota juuri käytettiin etukierroksen aikana.
Vaikka turvallisuusriskiä ei ole ja valmistaja voi luoda uuden tilauksen ja hyväksyä tapahtuman etukäteen, tämä hyökkäys voi varmasti vaikuttaa sallittujen tilausten käytettävyyteen. Motivoitunut hyökkääjä voisi todellakin estää kaikki sallitut käskyt tällä hyökkäyksellä. Harkitse vahvistamista, jos lupa on jo toimitettu tai jos määräraha riittää, tilauksen täyttövaiheessa. Harkitse myös kertomista käyttäjille tästä mahdollisesta hyökkäyksestä tilauksen laadinnan aikana.
Päivitys: Ei korjattu. 1 tuuman joukkue toteaa:
Meillä oli aiemmin hyväksyntätarkastuksia, mutta päätimme yksinkertaistaa lupavirtaa palataksemme vain epäonnistuneisiin hyväksyntöihin. Mietitään tapoja ilmoittaa tekijöille ongelmasta.
[L03] Kaksoiskoodi
Koodikannassa on kopioitua koodia. Koodin kopioiminen voi johtaa ongelmiin myöhemmin kehitysvaiheessa ja jättää projektin alttiimmaksi virheille. Tällaisia virheitä voi vahingossa ilmaantua, kun toiminnallisuuden muutoksia ei toisteta kaikissa koodin esiintymissä, joiden pitäisi olla identtisiä. Esimerkkejä kopioiduista koodeista ovat:
Koodin monistamisen sijaan harkitse vain yhden sopimuksen tai kirjaston käyttöä, joka sisältää monistetun koodin, ja käytä sitä aina, kun monistettua toimintoa tarvitaan.
Päivitys: Osittain kiinteä vetopyyntö # 60.
[L04] Virheellinen tai harhaanjohtava testisarja
Testisarjassa on tapauksia, joissa testit poikkeavat odotetusta käyttäytymisestään. Esimerkiksi:
- -
ChainlinkCalculator
sopimuksen periiOrderMixin
sopimus. Kuitenkin testien aikanaAmountCalculator.arbitraryStaticCall
-toimintoa käytetään kutsumaanChainlinkCalculator
sopimus ulkoisena itsenäisenä sopimuksena. Vaikka tulos on odotettu, testin tulisi heijastaa käyttäytymistä järjestelmän nykyisellä suunnittelulla ja odotetulla käyttötapauksella kutsumallaChainlinkCalculator
toimii suoraan ilman mielivaltaista staattista kutsua. - Vaikka välityssopimukset eivät kuuluneet soveltamisalaan, huomasimme, että testattaessa protokollaa ERC721-resurssilla
ERC721Proxy
sopimusta ei käytetä sen omaisuuden vaihtamiseen testisarja.
Koska itse testipaketti ei kuulu tämän tarkastuksen piiriin, harkitse testisarjan perusteellista tarkistamista varmistaaksesi, että kaikki testit suoritetaan onnistuneesti protokollan määritysten mukaisesti.
Päivitys: Kiinteä sisään vetopyyntö # 57, vetopyyntö # 59ja vetopyyntö # 61.
[L05] Tapahtumien virheet ja puutteet
Koko koodikannan aikana tapahtumia lähetetään yleensä, kun sopimuksiin tehdään arkaluonteisia muutoksia. Monista tapahtumista puuttuu kuitenkin indeksoituja parametreja ja/tai tärkeitä parametreja. Esimerkiksi:
On myös arkaluonteisia toimintoja, joista puuttuu tapahtumia, kuten:
Harkitse olemassa olevien tapahtumien täydellisempää indeksointia ja uusien parametrien lisäämistä, jos niitä puuttuu. Harkitse myös kaikkien tapahtumien lähettämistä niin täydellisesti, että niitä voitaisiin käyttää sopimuksen tilan palauttamiseen ketjun ulkopuolisilla palveluilla.
Päivitys: Ei korjattu. 1 tuuman joukkue kuitenkin lisäsi orderRemaining
parametrille OrderCanceled
tapahtuma sisään vetopyyntö # 62.
1 tuuman joukkue toteaa:
Huomasimme, että käyttöliittymän tarpeiden tyydyttämiseen tarvitaan vain rajoitettu osa datasta. Laajan analyysin tapauksessa kaikki ehdotetut kentät ovat käytettävissä jäljityksen kautta. varten
OrderRFQMixin
Odotamme markkinatakaajien rakentavan oman hienostuneen tavan seurata peruutettuja tilauksia.
[L06] Tallennusmuutokset tapahtumalähetyksen aikana
In NonceManager
sopimus, kun NonceIncreased
tapahtuma lähetetään, myös viestin lähettäjän nonce lisääntyy.
Useiden toimintojen suorittaminen samanaikaisesti voi tehdä koodikannasta vaikeampaa järkeillä, alttiimmaksi virheille ja voi johtaa toimintojen huomiotta jättämiseen tai väärinymmärrykseen.
Parantaaksesi koodin yleistä tarkoituksellisuutta, luettavuutta ja selkeyttä harkitse nonce-arvon kasvattamista ennen tapahtuman lähettämistä.
Päivitys: Kiinteä sisään vetopyyntö # 63.
[L07] Epäjohdonmukaiset dekoodausmenetelmät voivat aiheuttaa eroja lopputuloksissa
Kaiken laajennettavuuden ja joustavuuden tukemiseksi Limit Order Protocol -protokollan on rutiininomaisesti käsiteltävä dynaamisia tavutietoja ja mielivaltaisia ulkoisten sopimusten palautusarvoja. Tämän seurauksena protokolla sisältää an ArgumentsDecoder
kirjasto muuntaa tehokkaammin dynaamiset tavuarvot perustietotyypeiksi. Tätä kirjastoa ei kuitenkaan käytetä yksinomaan ja joissakin tapauksissa abi.decode
käytetään sen sijaan. Lisäksi jotkut sopimukset ovat käytössä abi coder v1
kun muut käyttävät abi coder v2
. Edellinen toimii enemmän kuin ArgumentsDecoder
kirjasto, kun taas jälkimmäinen suorittaa lisätarkistuksia purettaessa.
Näiden erilaisten dekoodausmenetelmien epäjohdonmukainen käyttö voi johtaa hienovaraisiin eroihin koodikannan tarkoituksen ja todellisen toiminnan välillä.
Esimerkiksi simulateCalls
toiminto käyttää vain ArgumentsDecoder.decodeBool
toiminto. Jos simulateCalls
-funktiota käytetään tarkistamaan kutsut, jotka tehtäisiin tilauksen predikaattiosassa, jolloin sen tulokset voivat poiketa siitä, mitä todella tapahtuu predikaattiehtoja arvioitaessa, koska käytetään erilaisia dekoodausmenetelmiä.
Joten esimerkiksi jos predikaatti tekee ulkoisen staticcall
johonkin funktioon, joka palauttaa a uint256
arvo on suurempi kuin yksi odotettua enemmän bool
, puhelu palautuu, koska palautusarvo on dekoodattu kanssa abi coder v2
n abi.decode
joka ei hyväksy sellaisia arvoja kuin bool
. Kuitenkin, jos täsmälleen sama puhelu soitetaan kanssa simulateCalls
, sitten se merkitään vain nimellä true
, Koska decodeBool
käsittelee kaikkia nollaa suurempia arvoja arvoina true
.
Jotta simulateCalls
funktio heijastaa täysin todellisten predikaattikutsujen käyttäytymistä, harkitse sen muokkaamista käytettäväksi abi.decode
.
Päivitys: Kiinteä sisään vetopyyntö # 82.
[L08] Syötteen validoinnin puute
- fillOrderToWithPermit
ja fillOrderTo
- toiminnot OrderMixin
sopimus sekä fillOrderRFQToWithPermit
ja fillOrderRFQTo
- toiminnot OrderRFQMixin
sopimusta, älä vahvista target
osoiteparametri.
Tämä mahdollistaa sen, että käyttäjä voi vahingossa välittää nollaosoitteen ja sen seurauksena lukita omaisuudet, jotka heidän on tarkoitus vastaanottaa tilauksen täyttämisen jälkeen.
Varmista, etteivät käyttäjät vahingossa lukitse varojaan, tarkistamalla, että target
osoite ei ole sama kuin nollaosoite viitatuissa funktioissa.
Päivitys: Kiinteä sisään vetopyyntö # 78.
[L09] Matala yksikkötestin kattavuus
Koko projektin yksikkötestauksen kattavuus on noin 75 %, ja osa sopimuksista on erityisen alhaista.
Ottaen huomioon yksikkötestien tärkeyden koodin validoinnissa ja regressioiden estämisessä uusien ominaisuuksien uudelleenmuodostuksessa ja kehittämisessä, suosittelemme kasvattamaan merkittävästi yksikkötestin kattavuutta vähintään 95 prosenttiin ja sisällyttämään reunatapaukset, jotka kattavat epätodennäköisetkin tilanteet.
Päivitys: Ei korjattu.
[L10] Harhaanjohtava tai puutteellinen sisäinen dokumentaatio
Koko koodikannan aikana tunnistettiin muutamia harhaanjohtavia ja/tai epätäydellisiä sisäisiä asiakirjoja, ja ne pitäisi korjata.
Seuraavat ovat harhaanjohtavia sisäisiä asiakirjoja:
- In
ChainlinkCalculator
sopimussinglePrice
funktion NatSpec@notice
tag sanoo että seCalculates price of token relative to ETH scaled by 1e18
, mutta itse asiassa sen tulos on arvo ofamount
skaalata rahakkeita1e18
, jossa oraakkeli ei välttämättä raportoi ETH:na (esimerkiksi parille, joka ei sisällä ETH:ta). - In
OrderRFQMixin
sopimusinvalidatorForOrderRFQ
funktion NatSpec@return
tag on harhaanjohtava, koska lainausta ei ehkä ole täytetty, jotta vastaava mitätöintibitti olisi asetettu. Tilaus on voitu myös peruuttaa. - Linjoilla 147, 165ja 188 of
OrderMixin.sol
, NatSpec@param
tagit ovat kieliopittomia. - On line 20 of
ERC1155Proxy.sol
, The@notice
-tunniste kertoo, että laskettu hajautus on tulosfunc_733NCGU
toiminto, missä sen pitäisi ollafunc_301JL5R
toiminto sen sijaan.
Seuraavat ovat epätäydellisiä sisäisiä dokumentaatioita:
- Toiminnot
AmountCalculator
sopimus ei kuvaa mitään parametreja. - In
ChainlinkCalculator
sopimussinglePrice
jadoublePrice
funktiot eivät kuvaa kaikkia parametreja. - In
ImmutableOwner
sopimus, julkisella muuttujalla ja muuntajalla ei ole NatSpec. - In
InteractiveNotificationReceiver
sopimusnotifyFillOrder
toiminto ei kuvaa mitään parametreja. - In
LimitOrderProtocol
sopimusDOMAIN_SEPARATOR
funktiolla ei ole NatSpec. - Tapahtumat ja kartoitukset alueella
NonceManager
ei ole NatSpec. - In
OrderRFQMixin
sopimus,cancelOrderRFQ*
funktiot eivät kuvaa palautusarvoja. - In
OrderMixin
sopimus, useista toiminnoista puuttuu täydellinen NatSpec. - On line 168 of
OrderMixin.sol
ja verkossa 71 ofOrderRFQMixin.sol
, siitä puuttuu@dev
tunnisteita. - Toiminnot
PredicateHelper
sopimuksessa ei kuvata kaikkia parametreja.
Selkeä sisäinen dokumentaatio on olennaista koodin tarkoitusten hahmottamisessa. Sisäisen dokumentaation ja toteutuksen väliset ristiriidat voivat johtaa vakaviin väärinkäsityksiin siitä, miten järjestelmän odotetaan käyttäytyvän. Harkitse näiden virheiden korjaamista, jotta kehittäjät, käyttäjät ja tarkastajat eivät aiheuta hämmennystä.
Päivitys: Osittain korjattu. Harhaanjohtavat asiakirjat käsitelty vetopyyntö # 75 ja vetopyyntö # 77.
1 tuuman joukkue toteaa:
Olemme korjanneet harhaanjohtavat asiakirjat. Asiakirjat viimeistellään myöhemmin.
[L11] DoS-tilaukset mahdollisia koukkuja käytettäessä
- OrderMixin
sopimus toteuttaa toiminnallisuuden yleisten off-chain swap-tilausten täyttämiseksi, joilla voi olla ehtoja niiden onnistumiselle. Tilauksen täytön aikana tilaus voi tarkista ennalta määritetyt predikaattiehdot ennen kuin jatkat suorittamista.
Koska nämä predikaattiehdot voivat kuitenkin kohdistua minkä tahansa mielivaltaisen sopimuksen logiikkaan, pahantahtoinen valmistaja voi huijata vastaanottajia uskomaan, että tilaus toimii oikein ja että se on pätevä, kun se tarkistetaan ketjun ulkopuolelta, mutta epäonnistuu silloin, kun yrittää täyttää saman tilauksen. ketjussa. Tämä muutos predikaattikäyttäytymisessä voidaan tehdä joko ajamalla eteen jotain muuttuvaa tilaa, josta predikaatit riippuvat, tutkimalla lähetettyä kaasua tai jopa mitkä osoitteet ovat mukana kutsussa, tai jollain muulla logiikalla.
Lisäksi, jos valmistaja määritteli a vuorovaikutusta vaihdon aikana, The interactionTarget
sopimus voisi peruuttaa itsensä tai peruuttaa lisäyksen estääkseen onnistuneen tilauksen täyttämisen, mikä johtaa olennaisesti samaan tulokseen kuin haitalliset predikaatit.
Vaikka omaisuus ei ole vaarassa, suotuisan tilauksen löytävillä käyttäjillä tai roboteilla on lisääntynyt taakka yrittää tunnistaa tällaisia roskapostitilauksia, jotka voivat näyttää pinnalta laillisilta. Jos he eivät tunnista tällaisia tilauksia, he maksavat hukkaan heitettyjä kaasukustannuksia. Vähentääksesi roskapostitilausten määrää, harkitse näiden koukkujen käytettävissä olevien kohteiden rajoittamista. Harkitse myös käyttäjien varoittamista tästä mahdollisuudesta ennen kuin he yrittävät täyttää tilauksia.
Päivitys: Ei korjattu. 1 tuuman joukkue toteaa:
Käsittelemme sen taustallamme ja pohdimme tapoja ilmoittaa mahdollisille vastaanottajille ongelmasta.
[L12] Pyöristys voi olla epäedullista taker
In OrderMixin
ja OrderRFQMixin
sopimukset, kun tilausta täytetään ja vastaanottaja toimittaa vain a makingAmount
or takingAmount
summa, protokolla yrittää laskea swapin vastasumman.
Näissä laskelmissa on kaksi ongelmaa, joista ensimmäinen on se, että ei ole olemassa dokumentaatiota tai logiikkaa, joka rajoita desimaalien määrää, jota summaparametrien tulee käyttää, mitä käsittelimme Dokumentoimattomat desimaalioletukset ongelma.
Toinen ongelma on, että näiden laskelmien aikana protokolla pyörii valmistajan eduksi. Pyöristysongelma voi pahentua huomattavasti, kun implisiittiset desimaalioletukset rikotaan, mutta vaikka kaikki olisi odotetusti, pyöristäminen tapahtuu pienillä, parittomilla summilla.
Harkitse, että annat vastaanottajan määrittää vähimmäismäärän makerAsset
omaisuuserä, jonka he ovat valmiita vastaanottamaan yhdessä enimmäismäärän kanssa takerAsset
he ovat valmiita vaihtamaan omaisuuserän, jotta pyöristyksen hyväksyminen on selkeämpää.
Päivitys: Ei korjattu. 1 tuuman joukkue toteaa:
Kynnysmäärän pitäisi riittää ottajan suojaksi.
[L13] Ristiriitainen tilausten käsittely parametrien puuttuessa
In OrderMixin
sopimus fillOrderTo
toiminto soittaa sisäisiä puheluita _callGetMakerAmount
ja _callGetTakerAmount
toimii aina, kun täyttöä yritetään ja joko makingAmount
tai takingAmount
parametrit ovat nolla, tai jos makingAmount
arvo on suurempi kuin remainingMakerAmount
arvoa.
- _callGetMakerAmount
ja _callGetTakerAmount
puhelut johtavat palautuksiin, jos tilausta ei luotu getMakerAmount
or getTakerAmount
parametrit, ja osittainen täyttö suoritetaan.
An upotettu kommentti rinnalla _callGetMakerAmount
ja upotettu kommentti rinnalla _callGetTakerAmount
väittävät, että "vain kokonaiset täytöt ovat sallittuja", jos tilausta ei luotu getMakerAmount
or getTakerAmount
parametreja.
On kuitenkin koodipolkuja, joihin tämä ei päde, koska nämä polut eivät tarkista length
s molemmista getMakerAmount
ja getTakerAmount
parametreja.
Erityisesti, kun taker
määrittelee a takerAmount
arvo tilaukselle, jolla on vain a getMakerAmount
, ellei se kutsu getMakerAmount
palauttaa suuremman summan kuin remainingMakerAmount
, osittainen täyttö voidaan suorittaa vastoin tekstin sisäistä dokumentaatiota.
Tämä jättää näiden koodipolkujen tarkoituksellisuuden epäselväksi. Jos tämä on odotettu toiminta, harkitse tekstin sisäisen dokumentaation muokkaamista selvemmäksi. Jos tämä on tahatonta toimintaa, harkitse aina molempien pituuksien tarkistamista getMakerAmount
ja getTakerAmount
parametreja samanaikaisesti, jotta toteutus vahvistaa sisäisissä dokumentaatioissa kuvattua käyttäytymistä.
Päivitys: Kiinteä sisään vetopyyntö # 79.
[L14] Käytöstä poistettujen Chainlink-puheluiden käyttö
- ChainlinkCalculator
sopimus on tarkoitettu käytettäväksi Chainlink-oraakkeleiden kyselyyn. Se tekee sen soittamalla heille latestTimestamp
ja latestAnswer
menetelmiä, jotka molemmat on poistettu käytöstä. Itse asiassa menetelmät eivät enää ole Chainlink-aggregaattoreiden API:ssa versiosta kolme.
Voit välttää mahdolliset tulevat yhteensopimattomuudet Chainlink-oraakkelien kanssa harkitsemalla latestRoundData
menetelmän sijaan.
Päivitys: Kiinteä sisään vetopyyntö # 67.
Huomautuksia ja lisätietoja
[N01] Ei tuo liitäntöjä
- AggregatorInterface
käyttöliittymä näyttää olevan koodin osajoukko, josta on kopioitu ChainLink
julkinen koodivarasto. Täysi käyttöliittymä sisältyy ChainLink
sopimuksen npm-paketti.
Harkitse niiden virallisten npm-pakettien kautta asennettujen liitäntöjen käyttöä mahdollisuuksien mukaan rajapintojen epäsuhtaisuuden ja niistä aiheutuvien ongelmien vähentämiseksi sen sijaan, että määrittelisit ja/tai kirjoittaisit uudelleen toisen projektin rajapintoja.
Päivitys: Kiinteä sisään vetopyyntö # 66.
[N02] Vanhentuneet projektiriippuvuudet
Asennuksen aikana projektin riippuvuudet, NPM varoittaa, että yksi asennetuista paketeista, Highlight
, "ei enää tueta tai vastaanota tietoturvapäivityksiä tulevaisuudessa".
Vaikka on epätodennäköistä, että tämä paketti voisi aiheuttaa tietoturvariskin, harkitse tätä pakettia käyttävän riippuvuuden päivittämistä ylläpidetyksi versioon.
Päivitys: Kiinteä sisään vetopyyntö # 64.
[N03] Ulkoiset kutsut tarkastella menetelmiä eivät ole staattisia kutsuja
Suurimmassa osassa koodikantaa protokolla tekee eksplisiittisesti ulkopuheluita OpenZeppelinin kautta. functionStaticCall
menetelmä rajoittaa tilamuutosten mahdollisuutta, kun niitä ei joko odoteta tai ei toivota. Kuitenkin vuonna ChainlinkCalculator
sopimusta, vaikka aikomus tehdä ulkopuheluita vain view
menetelmät Chainlink oraakkeleissa, ulkoiset puhelut singlePrice
ja doublePrice
toimintoja ei tehdä eksplisiittien kautta staticcall
s.
Vaikka emme havainneet välittömiä tästä johtuvia turvallisuusongelmia, hyökkäyspinnan vähentämiseksi, johdonmukaisuuden parantamiseksi ja tarkoituksen selkeyttämiseksi harkitse nimenomaisen staticcall
s, kaikille ulkoisille puheluille view
toiminnot ChainlinkCalculator
sopimus.
Päivitys: Ei korjattu. 1 tuuman joukkue toteaa:
Uskomme, että syntaksin monimutkaisuus mitätöi johdonmukaisuuden parannukset.
[N04] Epäonnistuminen aikaisin virheellisten tilausten kanssa
In OrderMixin
sopimus fillOrderTo
toiminto käsittelee erikoistilan, kun tilausta ei ole aiemmin lähetetty (remainingMakerAmount == 0
), mutta se ei nimenomaisesti käsittele ehtoa, kun tilaus ei ole enää voimassa (remainingMakerAmount == 1
).
Jälkimmäisessä skenaariossa toiminto palautuu lopulta, mutta vasta sen jälkeen, kun ei-triviaaleja kaasumääriä on poltettu. Tarkoituksenmukaisuuden selkeyttämiseksi, luettavuuden parantamiseksi ja kaasun käytön vähentämiseksi harkitse virheellisen tilauksen skenaarion nimenomaista käsittelemistä toiminnon alussa.
Päivitys: Kiinteä sisään vetopyyntö # 68.
[N05] Auttajasopimuksia ei ole merkitty abstrakteiksi
Solidityssä avainsana abstract
käytetään sopimuksiin, jotka eivät ole itsessään toimivia sopimuksia tai joita ei ole tarkoitettu käytettäväksi sellaisenaan. Sen sijaan, abstract
sopimukset perivät muut järjestelmän sopimukset luodakseen itsenäisiä toiminnallisia sopimuksia.
Kaikkialla koodikannassa on esimerkkejä avustajasopimuksista, joita ei ole merkitty abstrakteiksi, vaikka niitä ei ole tarkoitus ottaa käyttöön yksinään. Esimerkiksi, AmountCalculator
, ChainlinkCalculator
, ImmutableOwner
, NonceManager
ja PredicateHelper
Kaikki sopimukset koostuvat perusjoukosta toimintoja, jotka on tarkoitettu käytettäväksi perintösopimuksissa.
Harkitse avustajasopimusten merkitsemistä abstract
osoittamaan selvästi, että ne on suunniteltu ainoastaan lisäämään toimintoja sopimuksiin, jotka perivät ne.
Päivitys: Ei korjattu. 1 tuuman joukkue toteaa:
Nämä avustajat voidaan ottaa käyttöön erikseen. Ne peritään vain kaasun säästämiseksi.
[N06] Epäjohdonmukainen toimintojärjestys
Kaikkialla koodikannassa funktioiden järjestys noudattaa yleensä suositeltu tilaus Solidity Style Guide -oppaassa, mikä on: constructor
, fallback
, external
, public
, internal
, private
.
Kuitenkin vuonna OrderMixin
sopimus public
checkPredicate
funktio poikkeaa tyylioppaasta ja puolittaa sen external
toiminnot.
Parantaaksesi projektin yleistä luettavuutta, harkitse toimintojen järjestyksen standardointia koko koodikannassa Solidity Style Guide -oppaan suositusten mukaisesti.
Päivitys: Kiinteä sisään vetopyyntö # 69.
[N07] Epäjohdonmukainen tilauksen täyttövirta
- OrderMixin
ja RFQOrderMixin
Molemmat sopimukset hoitavat allekirjoitettujen tilausten täyttämisen, mutta yleinen tilausvirta kahden sopimuksen välillä on epäjohdonmukainen.
OrderMixin
n fillOrderTo
funktio seuraa tätä yleistä kulkua (pseudokoodi):
if ((takingAmount == 0) == (makingAmount == 0))
else if (takingAmount == 0)
else (handle makingAmount == 0) THEN swapTokens
taas RFQOrderMixin
on analoginen fillOrderRFQTo
toiminto seuraa tätä kulkua (pseudokoodi):
if (takingAmount == 0 && makingAmount == 0)
else if (takingAmount == 0)
else if (makingAmount == 0)
else revert THEN swapTokens
Dokumentaatiosta ei löydy oivalluksia siitä, miksi ensimmäinen ehdollinen kummassakin näistä kahdesta funktiosta eroaa tai miksi takingAmount
ja makingAmount
molemmat eivät voi olla nollia jälkimmäisessä funktiossa. Myös tapaus, jossa molemmat a makingAmount
ja takingAmount
on paljon helpompi perustella fillOrderRFQTo
toiminto, koska se käsitellään selvästi finaalissa else
lohko.
Tarkoituksena on selventää ja parantaa koodin yleistä luettavuutta harkitsemalla joko näiden kahden sopimuksen yleisen tilausvirran standardointia tai selkeästi dokumentoimalla erojen syyt.
Päivitys: Ei korjattu. 1 tuuman joukkue toteaa:
Tämä johtuu rajatilausten mukautetuista hinnoittelutoiminnoista. Siitä asti kun
getMakerAmount
voi mahdollisesti poiketa olennaisestigetTakerAmount
, ajattelimme, että on parempi olla tekemättä oletusasetusta ottajalle, koska se luultavasti hämmentää heitä tapauksissa, joissa saattajat ovat erilaisia.
[N08] Virheilmoitukset ovat epäjohdonmukaisesti muotoiltuja tai harhaanjohtavia
Kaikkialla koodikannassa require
ja revert
virheviestit, joiden tarkoituksena on ilmoittaa käyttäjille tietyistä olosuhteista, jotka aiheuttavat tapahtuman epäonnistumisen, todettiin muotoiltuiksi epäjohdonmukaisesti.
Esimerkiksi jokainen rivillä olevista virheilmoituksista 85 of OrderMixin.sol
, 16 of ERC721ProxySafe.sol
ja 26 of Permitable.sol
käyttää eri tyyliä.
Lisäksi jotkin virheilmoitukset ovat harhaanjohtavia:
Virheviestien tarkoituksena on ilmoittaa käyttäjille epäonnistuneista olosuhteista, joten niiden tulee antaa riittävästi tietoa, jotta tarvittavat korjaukset voidaan tehdä vuorovaikutuksessa järjestelmän kanssa. Epätietoiset virheilmoitukset vahingoittavat suuresti yleistä käyttökokemusta ja heikentävät siten järjestelmän laatua. Lisäksi epäjohdonmukaisesti muotoillut virheilmoitukset voivat aiheuttaa tarpeetonta sekaannusta. Siksi harkitse koko koodikannan tarkistamista varmistaaksesi, että jokainen require
ja revert
Lausunnossa on virheilmoitus, joka on johdonmukaisesti muotoiltu, tarkka, informatiivinen ja käyttäjäystävällinen.
Päivitys: Osittain kiinteä vetopyyntö # 81.
[N09] Nimettyjen palautusmuuttujien epäjohdonmukainen käyttö
Nimettyjä palautusmuuttujia käytetään epäjohdonmukaisesti OrderMixin
sopimus. Jotkut toiminnot palauttaa nimetyt muuttujat, muut palauttaa eksplisiittiset arvot, ja muut ilmoittaa nimetty palautusmuuttuja, mutta ohittaa sen selkeällä paluulauseella.
Harkitse johdonmukaista lähestymistapaa palauttaa arvoja koko koodikannassa poistamalla kaikki nimetyt palautusmuuttujat, ilmoittamalla ne paikallisiksi muuttujiksi ja lisäämällä tarvittavat palautuslausekkeet tarvittaessa. Tämä parantaisi sekä koodin selkeyttä että luettavuutta, ja se voi myös auttaa vähentämään regressioita tulevien koodimuutosten aikana.
Päivitys: Kiinteä sisään vetopyyntö # 73.
[N10] Tilauksen hash-laskenta ei ole avoin API:lle
- external
tehtävät remaining
, remainingRaw
ja remainingsRaw
kaikki odottavat tilaustiivistettä onnistuneesta toiminnasta.
Kuitenkin aputoiminto _hash
, joka palauttaa tilauksen hashin, on private
näkyvyys. Tämä tarkoittaa, että käyttäjien on pakattava osa tilauksista ja verkkotunnusmerkkijonoista manuaalisesti saadakseen tilauksen tiivisteen.
Jotta vältytään mahdollisilta virheiltä tilaustiivistettä laskettaessa ja jotta käyttäjille tarjotaan menetelmä tilauksen vastaavan tiivisteen luomiseksi, harkitse tilauksen näkyvyyden laajentamista. _hash
toimimaan public
ja nimen muokkaaminen uudelleen muotoon hash
olla johdonmukainen muun koodin kanssa.
Päivitys: Kiinteä sisään vetopyyntö # 74.
[N11] Kartoituksen semanttinen ylikuormitus
- _remaining
kartoitus OrderMixin
sopimus on semanttisesti ylikuormitettu tilausten tilan ja kyseisiin tilauksiin käytettävissä olevan omaisuuden seuraamiseksi.
Kolme tilaa, jotka se voi ottaa, ovat:
0
: Tilaustiivistettä ei ole vielä nähty.1
: Tilaus on joko peruutettu tai täytetty kokonaan.- mitään
uint
suurempi kuin1
: Jäljellä olevamakerAmount
voidaan täyttää tilaus plus1
.
Tämä semanttinen ylikuormitus vaatii tämän arvon käärimistä ja purkamista aikana lookup
, cancellation
, initialization
ja storage
.
Semanttinen ylikuormitus ja sen mahdollistava logiikka voivat olla alttiita virheille ja tehdä koodikannasta vaikeampaa ymmärtää ja perustella, se voi myös avata oven regressioille tulevissa koodin päivityksissä.
Parantaaksesi koodin luettavuutta, harkitse tilausten valmistumistilan seurantaa erillisessä kartoituksessa.
Päivitys: Ei korjattu. 1 tuuman tiimi mainitsi, että korjaus nostaisi kaasukustannuksia fillOrder
toiminto.
[N12] Luvalliset tilaukset sallivat mielivaltaisten sopimusten kutsumisen
- OrderMixin
sopimus perii Permitable
sopimus mahdollistaa yksittäisen tapahtuman tilauksen täyttämisen sellaisilla varoilla, jotka hyväksyvät tällaisen permit
kehottaa muuttamaan päästöoikeuksia.
Kuitenkin, puhelut osoitteeseen Permitable
sopimus älä vahvista, onko kohde sallittu omaisuus, tai onko se edes omaisuus, joka voi sallia haitallisen käyttäjän välittää mielivaltaisen sopimuksen osoitteen, joka voisi suorittaa toisen kutsun ennen tilauksen täyttöä.
Vaikka sopimus on suojattu paluuta vastaan, hyökkäyspinnan vähentäminen ja ulkoisten sopimusten kutsumisen estäminen suorituksen aikana on aina suositeltavaa. Harkitse luvan sisältämän sisällön rajoittamista tilaukseen liittyviin kohteisiin tai protokollan sallittujen sisältöjen luetteloon.
Päivitys: Ei korjattu. 1 tuuman joukkue toteaa:
OrderMixin
ei itse asiassa ole tietoa todellisista tokeneistamakerAsset
jatakerAsset
joskus ovat välityspalvelimia tai muita välisopimuksia ja tiedot todellisista tokeneista on tallennettu joihinkin mielivaltaisiin tavuihin. Joten ei ole järkevää tapaa rajoittaa mitä omaisuuttapermit
kutsutaan.
[N13] solhint
ei ole koskaan uudelleen käytössä
Koko koodikannassa on pari solhint-disable
lausunnot, erityisesti verkossa olevat 23 ja verkossa 41 of RevertReasonParser.sol
, joita ei ole päätetty solhint-enable
.
Puolustaa selkeyttä ja olla mahdollisimman rajoittava vammautumisen yhteydessä solhint
, harkitse käyttöä solhint-disable-line
or solhint-disable-next-line
sen sijaan samanlainen kuin rivi 16 samasta tiedostosta.
Päivitys: Kiinteä sisään vetopyyntö # 72.
[N14] Kirjoitusvirheet
Koodikanta sisältää seuraavat kirjoitusvirheet:
Lisäksi projektin README
(ei kuulu tähän tarkastukseen) sisältää seuraavat kirjoitusvirheet:
Harkitse näiden kirjoitusvirheiden korjaamista koodin luettavuuden parantamiseksi.
Päivitys: Kiinteä sisään vetopyyntö # 71 ja vetopyyntö # 77.
[N15] Käyttö uint
sijasta uint256
Yksinkertaisuuden suosimiseksi, kaikki tapaukset uint
tulee ilmoittaa uint256
. Erityisesti ne for
silmukat linjoilla 98 ja 119 of OrderMixin.sol
ja linjat 16 ja 30 of PredicateHelper.sol
.
Päivitys: Kiinteä sisään vetopyyntö # 70.
Päätelmät
3 erittäin vakavia ongelmia havaittiin. Joitakin muutoksia ehdotettiin parhaiden käytäntöjen noudattamiseksi ja mahdollisen hyökkäyksen vähentämiseksi.
- &
- 7
- Meistä
- pääsy
- Mukaan
- Tili
- poikki
- Toimia
- toimet
- lisä-
- osoite
- Etu
- Kaikki
- Salliminen
- jo
- Vaikka
- määrät
- analyysi
- api
- lähestymistapa
- perustelut
- noin
- etu
- Varat
- tilintarkastus
- Back-end
- Alku
- ovat
- PARAS
- parhaat käytännöt
- Bitti
- botit
- rakentaa
- soittaa
- joka
- tapauksissa
- Aiheuttaa
- chainlink
- muuttaa
- tarkkailun
- Tarkastukset
- koodi
- monimutkainen
- ehto
- sekaannus
- rakentaminen
- sisältää
- sopimus
- sopimukset
- Korjaukset
- kustannukset
- voisi
- Pari
- luoja
- valuutta
- Nykyinen
- tiedot
- sopimus
- Palvelunesto
- levityspinnalta
- Malli
- kehittäjille
- Kehitys
- DID
- erota
- eri
- verkkotunnuksen
- kaksinkertainen
- dynaaminen
- Varhainen
- reuna
- kannustaa
- erityisesti
- ETH
- tapahtuma
- Tapahtumat
- kaikki
- esimerkki
- Vaihdetaan
- odotettu
- experience
- Käyttää hyväkseen
- Ominaisuudet
- Fields
- Vihdoin
- Etunimi
- Korjata
- Joustavuus
- virtaus
- seurata
- löytyi
- koko
- toiminto
- varat
- tulevaisuutta
- peli
- GAS
- general
- Antaminen
- suuri
- ohjaavat
- Käsittely
- hasis
- hajautusta
- ottaa
- auttaa
- Korkea
- erittäin
- Miten
- HTTPS
- Hybridi
- tunnistaa
- Vaikutus
- toteuttaa
- tärkeä
- tuovan
- mukana
- Mukaan lukien
- Kasvaa
- kasvoi
- tiedot
- tiedot
- Infrastruktuuri
- oivalluksia
- tahallisuus
- korko
- liitäntä
- osallistuva
- kysymykset
- IT
- suuri
- suurempi
- johtaa
- johtava
- Kirjasto
- rajallinen
- linja
- likviditeetti
- lueteltu
- Listat
- paikallinen
- Katsoin
- Katso ylös
- merkittävä
- valmistaja
- Tekeminen
- markkinat
- Mempool
- peili
- malli
- eniten
- Suosituin
- nimittäin
- Uudet ominaisuudet
- ei-korvattavissa
- ei-fungible tokens
- virallinen
- avata
- Operations
- Vaihtoehto
- oraakkeli
- tilata
- määräys
- Muut
- omistaja
- Kuvio
- Suosittu
- esittää
- estää
- hinta
- hinnoittelu
- yksityinen
- prosessi
- projekti
- hankkeet
- suojaus
- protokolla
- toimittaa
- tarjoaa
- valtuutettu
- julkinen
- julkaista
- osto
- laatu
- nostaa
- Todellisuus
- vähentää
- riippuvuus
- raportti
- Raportit
- säilytyspaikka
- vaatimukset
- REST
- tulokset
- Tuotto
- arviot
- Riski
- kierrosta
- ajaa
- sdk
- turvallisuus
- Palvelut
- setti
- yhteinen
- osakkeet
- siirtää
- samankaltainen
- Yksinkertainen
- pieni
- fiksu
- Smart-sopimukset
- So
- kiinteys
- spam
- erityisesti
- menot
- Kaupallinen
- Alkaa
- Osavaltio
- Lausunto
- Valtiot
- Tila
- Levytila
- tyyli
- toimitettu
- menestys
- onnistunut
- Onnistuneesti
- tuki
- Tuetut
- pinta
- Vaihtaa
- järjestelmä
- Kohde
- testi
- Testaus
- testit
- Kautta
- kauttaaltaan
- aika
- yhdessä
- symbolinen
- tokens
- raita
- Seuranta
- kauppa
- Luottamus
- unique
- Päivitykset
- us
- käytettävyys
- USD
- USDT
- Käyttäjät
- hyödyllisyys
- arvo
- Näytä
- näkyvyys
- odottaa
- Mitä
- whitelist
- sisällä
- ilman
- Referenssit
- arvoinen
- nolla-