December 16, 2021
Bevezetés
A 1inch csapat arra kért minket, hogy vizsgáljuk felül és auditáljuk Limit Order Protocol v2 intelligens szerződések. Megnéztük a kódot, és most közzétesszük eredményeinket.
Kör
Ellenőriztük a kötelezettségvállalást 4d94eea25e4dac6271bfd703096a5c4a4d899b4a
az 1inch/limit-order-protocol
adattár. A hatálya alá a következő szerződések vonatkoztak:
- 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
Az összes többi projektfájlt és -könyvtárat (beleértve a teszteket is), valamint a külső függőségeket és projekteket, a játékelméletet és az ösztönző tervezést szintén kizárták az ellenőrzés hatóköréből. Feltételezték, hogy a külső kód- és szerződésfüggőségek dokumentáltan működnek, az 1inch által nyújtott háttérszolgáltatások pedig a protokoll legjobb érdekeit szolgálják.
Általános egészségi állapot
Általánosságban úgy találtuk, hogy a projekt kódbázisa olvasható és jól szervezett, bár hasznos lehet a kiterjedtebb dokumentáció, különösen az összeállítási kód blokkjai, a protokoll szélső esetei, a felhasználandó eszközök/predikátumok/külső erőforrások, a nyújtott háttérszolgáltatás felelősségei/korlátai és a szereplők közötti interakciók. A projekt minden erőfeszítést megtesz annak érdekében, hogy a tevékenységeket gáztakarékossá tegye, esetenként még azzal a kockázattal is, hogy a kód nehezebben értelmezhetővé válik; az alábbiakban ezzel kapcsolatos kérdéseket vetünk fel. Az ellenőrzés során az 1 hüvelykes csapat rendkívül elérhető volt, készséges volt, és nagyon könnyű volt vele dolgozni.
Rendszer áttekintő
A Limit Order Protocol lehetővé teszi a rendelést makers
láncon kívüli megbízásokat aláírni token csereügyletekre. A jegyzőkönyv ezután megkönnyíti a korábban aláírt megbízások megrendeléssel történő kitöltését takers
. A megrendelések nagymértékben bővíthetők, és a rendeléskitöltési folyamat során több ponton is statikusan hívhatják a külső szerződéseket. Ez a bővíthetőség hasznossággal itatja át a protokollt, de egyszerre bonyolítja és nagyobb támadási felületet biztosít maguknak a rendeléseknek.
Fontos megjegyezni, hogy a megrendelés részleteinek nincs láncon belüli tárolása. A rendelések kitöltési vagy törlési állapotát a rendszer csak a rendeléskivonatokon keresztül követi nyomon. Ez szükségessé teszi, hogy a megrendeléseket peer-to-peer módon vagy egy központosított félen keresztül megosszák. Ebben az esetben az 1 hüvelykes csapat központi félként kíván fellépni, összesíti az aláírt megbízásokat, és ezeket a megbízásokat likviditási forrásként használja más protokolljaihoz. A rendeléseket a saját API-jukon keresztül teszik közzé, így a felhasználók interakcióba léphetnek velük.
Ez a központosítás az 1 hüvelykes csapat számára rendkívüli ellenőrzést biztosít a megbízások közzététele és végső végrehajtása felett. Ez lehetőséget ad számukra a parancsok cenzúrázására is, ami hasznos lehet rosszindulatú vagy megtévesztő megbízások esetén, de visszaélhetnek vele, és lehetővé teszik számukra, hogy bármely más felhasználót előkerítsenek egy kedvező rendelés esetén, ha nem jelenítik meg az API-n keresztül.
Kiváltságos szerepek
Bár a szerepkört felhasznált szerződések nem terjedtek ki a hatálya alá, egy kiemelt szerepkört azonosítottak. An immutableOwner
a meghatalmazotti szerződés létrehozójának van beállítva az építéskor, és arra szolgál, hogy korlátozza a hozzáférést a meghatalmazotthoz. external
funkciókat.
Külső függőségek és bizalmi feltételezések
Ennek a protokollnak a kialakítása láncon kívüli és láncon belüli komponenseket tesz szükségessé, és ez a hibrid modell felhasználható néhány, a jelentésünkben azonosított támadási vektor mérséklésére, de ennek a képességnek a költsége az 1 hüvelykes csapatra és infrastruktúrára való fokozott támaszkodás.
Ezenkívül a Limit Order Protocol olyan funkciókat biztosít, amelyek célja az árak lekérése a Chainlink orákulumokból. Feltételeztük, hogy ezek az orákumok becsületesek, hozzáférhetőek és megfelelően működnek.
Sőt, a megrendelés rugalmassága miatt számos olyan kapcsolati pont van a külső szerződésekkel, amelyeket nem érvényesítenek. Ez azt jelenti, hogy egy rosszindulatú felhasználó visszaélhet az ilyen hívásokkal, és rosszindulatú szerződésekkel megszemélyesítheti predikátumokat, eszközöket vagy orákulumokat, hogy műveleteket hajtson végre a rendelések kitöltése során. Bár a projekt bizonyos területeken védett az újrabelépés ellen, az ilyen vektorok szolgáltatásmegtagadási támadásokat vagy észrevétlen spam-megrendeléseket okozhatnak. Az 1 hüvelykes csapat tisztában van azzal, hogy bizonyos problémák merülhetnek fel, ha ismeretlen szerződéseket használnak a protokollhoz, és jelezték azon szándékukat, hogy a projekt csak a nagyobb „blue chip” eszközöket támogatja teljes mértékben. Meg kell azonban jegyezni, hogy még a legnépszerűbb eszközök esetében is vannak olyan belső viselkedési jellemzők, amelyek problémákat okozhatnak azokkal a protokollokkal, amelyek nem megfelelően kezelik őket, például az USDT-vel történő átutalás során díjat számítanak fel, vagy hibakódot küldenek vissza siker logikai érték cTokenekkel.
Álláspontja
Itt mutatjuk be megállapításainkat.
Kritikus súlyosság
Egyik sem.
Magas súlyosság
[H01] Inkonzisztens adatok kerültek át _makeCall
A OrderMixin
szerződés, a _makeCall
funkciót eszközök átvitelére használják az átvevőtől a készítőig és azután a készítőtől az átvevőig. Ez utóbbi transzfernél a _makeCall
funkciót hibásan adták át a megbízásnak makerAsset
utolsó paraméterként, amikor a sorrendben kell lennie makerAssetData
.
Ennek eredményeként minden proxyfunkció, amely a makerAssetData
az érv megszakad.
Hogy összhangban legyen a korábbi felhívással _makeCall
és a proxy funkciók teljes körű támogatása érdekében fontolja meg a order.makerAsset
paraméter a order.makerAssetData
.
Frissítés: Rögzítve lehúzási kérés #57.
[H02] A részben kitöltött magánrendeléseket bárki teljesítheti
A protokoll lehetővé teszi magán- és állami megrendelések létrehozását. Magánrendelésre csak a allowedSender
A készítő által a megrendelés létrehozása során megadott címen a megrendelést teljesíteni tudja.
Azonban a OrderMixin
szerződés, érvényesítés a allowedSender
cím hatóköre helytelen, ami azt jelenti, hogy csak a rendelés első kitöltését kezelő logikán belül kerül kiértékelésre. Ha egy magánrendelés részben kitöltött, akkor a csekk a allowedSender
cím már nem elérhető, és a rendelés bárki által kitölthetővé válik.
Annak tisztázása érdekében, hogy bármely felhasználó képes legyen-e részben kitöltött magánrendeléseket kitölteni vagy sem, fontolja meg az aktuális viselkedés okának dokumentálását vagy a allowedSender
címen kívül esik az első kitöltés hatókörén, így biztosítva, hogy minden kitöltési kísérletkor érvényes legyen.
Frissítés: Rögzítve lehúzási kérés #58.
[H03] A rosszindulatú készítők kihasználhatják a részleges kitöltéseket, hogy ellopják az átvevő eszközeit
Megrendelések a OrderMixin
a szerződés részben kitölthető. A részleges kitöltések támogatásához a protokollnak szüksége van a csereügyletek mindkét oldalának kiszámítására. Mindkét getMakerAmount
és a getTakerAmount
mezőket a megrendelés készítője határozza meg pontosan erre a célra.
A megrendelés kitöltésekor az átvevőknek meg kell adniuk vagy a makingAmount
vagy a takingAmount
értékeket, valamint a thresholdAmount
érték. Két különböző kódút választható, attól függően, hogy a makingAmount
vagy a takingAmount
biztosítva volt.
Az első az, amikor a makingAmount
paraméter van megadva. Megtehetem csonka a makingAmount
értékét és azt is számolja ki a takingAmount
érték érte. Ebben a helyzetben a thresholdAmount
biztosítja, hogy a takingAmount
vett értéke nem váratlanul nagy.
A második az, amikor a takingAmount
paraméter definiálva van. Ilyen esetben az lesz számolja ki a makingAmount
értékkel, lehetőséggel csonkolva azt és a újraszámolva a takingAmount
érték, ha ez megtörténik. Ebben a helyzetben a thresholdAmount
érték biztosítja, hogy a makingAmount
a visszaadott érték nem váratlanul kicsi.
Két kihasználási módszer létezik, amelyek mindegyike egyedi az előzőekben említett kódútvonalak egyikére. Ezek a kihasználási módszerek rosszindulatúakat igényelnek getMakerAmount
és a getTakerAmount
funkciókat. Ezeknek a függvényeknek az egyszerű megvalósítása hasonlóképpen viselkedne AmountCalculator
„s getMakerAmount
és a getTakerAmount
funkciókat, de egy kemény kódolt kapcsolóval, amely arra kényszeríti őket, hogy szükség esetén a támadó által vezérelt értéket adjanak vissza.
Az első, kevésbé súlyos kihasználási minta magában foglalja az első kódútvonalat, ahol a makingAmount
érték van megadva kitöltési sorrendben. A rosszindulatú gyártó a kitöltési sorrendre vár, amely megadja makingAmount
hogy megjelenjen a mempoolban, hogy előfuthasson. Az 1 kivételével az összes értéket leeresztik a gyártó oldaláról, majd erőltetik _callGetTakerAmount
a felhasználó által megadott összeg visszaküldésére thresholdAmount
értékét (vagy pótlékukat, ha ez kisebb). Amikor a felhasználó tranzakciója végre megtörténik, a teljes összeget kicseréli thresholdAmount
értéke takerAsset
egyetlen egységére makerAsset
. Ezt a kihasználást az által megadott mennyiség korlátozza thresholdAmount
értékét vagy összegét a takerAsset
a felhasználó engedélyezte a LimitOrderProtocol
szerződés.
A második, súlyosabb kihasználási minta magában foglalja a második kódútvonalat, ahol a takingAmount
érték van megadva. A rosszindulatú készítő hasonlóképpen várna egy olyan kitöltési parancsot, amely a takingAmount
érték jelenik meg a mempoolban. Előre lebonyolítanák a tranzakciót és kényszerítenék a makingAmount
által visszaadott érték _callGetMakerAmount
függvény magasabb legyen mind a remainingMakerAmount
és a thresholdAmount
. Azt is beállítanák a takingAmount
által visszaadott értéket _callGetTakerAmount
hogy az összeg legyen takerAsset
eszközön engedélyezett LimitOrderProtocol
az átvevő által. Amikor az átvevő tranzakciója megtörténik, akkor meg fog történni csonkolja a makingAmount
érték, majd újraszámolja a takingAmount
érték. Ez az újraszámítás azonban nem garantáltan alacsonyabb lesz, és ebben az esetben kimeríti az átvevőt takerAsset
hogy engedélyezték a szerződésen. Ezen a kódútvonalon a thresholdAmount
érték az annak biztosítása, hogy a makingAmount
nem túl alacsony, tehát az összes elvevőt figyelembe véve takerAsset
az eszköz nincs bejelölve. Az elvesztett pénzeszközök összege a takerAsset
A felhasználó által engedélyezett eszköz a LimitOrderProtocol
szerződés.
Ezek a kihasználások nem lehetségesek részleges parancsok, pontosabban rosszindulatú részleges megrendelések nélkül getMakerAmount
és a getTakerAmount
megvalósítások.
A fő kérdés a thresholdAmount
Az értékellenőrzés az, hogy a csereügyletnek csak az egyik oldalát fedi le, de a másik oldal frontrunningen keresztül manipulálható. Nincs garancia arra, hogy az átvevő által eredetileg javasolt érték változatlan marad. Fontolja meg az eltávolítást makingAmount
mindkét kódútvonal csonkítása és visszaállítás, ha a rendelés nem támogatja a kért nagyságú kitöltést. Ezzel a thresholdAmount
használható a swap másik oldalának kellő korlátozására és a váratlan viselkedés elkerülésére, még rosszindulatú megbízások esetén is.
Frissítés: Rögzítve lehúzási kérés #83.
Közepes súlyosságú
[M01] A dinamikus argumentumok után átadott statikus argumentumok
A OrderMixin
szerződés, a getTakerAmount
és a getMakerAmount
bytes mezők argumentumaként szolgálnak a _callGetTakerAmount
és a _callGetMakerAmount
funkciókat. Ezek a hívások lehetőséget biztosítanak a csereügylet egyik oldalának a másik oldal alapján történő kiszámítására, és lehetővé teszik a felhasználók számára, hogy részben teljesítsék a rendeléseket.
A getTakerAmount
/getMakerAmount
A mezők dinamikus változók, és a mező elé vannak csomagolva takerAmount
és a makerAmount
értékek a _callGetTakerAmount
és a _callGetMakerAmount
funkciókat. Előfordulhat, hogy egy rosszindulatú készítő a vártnál több adatot szolgáltat a getTakerAmount
és agetMakerAmount
mezők tolni a takerAmount
és a makerAmount
bájtokkal túl van azon a helyen, ahol a következő függvényben dekódolják. Ez lehetővé teszi a készítő számára, hogy egy teljes bájttal jobbra tolja az átadott fogadó vagy készítő mennyiségét, és akár teljesen le is cserélje, ha további 32 bájt adatot biztosít.
A felhasználóknak már manuálisan át kell nézniük a getTakerAmount
és a getMakerAmount
mezőket a sorrendben, de ezt a technikát meglehetősen nehéz észrevenni. Érdemes megjegyezni, hogy ez a támadás még a belsőleg megbízható személyekre is vonatkozik getMakerAmount
és a getTakerAmount
funkciókat. A legtöbb támadás esetén egy ésszerű küszöbérték megadása megakadályozza a pénzeszközök elvesztését.
Ennek elkerülése érdekében fontolja meg a statikus argumentumok kódolását a dinamikus argumentumok előtt, nehogy a dinamikus argumentumoknak módszert adjon a statikus argumentumok vezérlésére.
Frissítés: Nincs kijavítva. Az 1 hüvelykes csapat kijelentette:
Különös gondot fordítunk a getterek érvényesítésére. Megpróbáljuk megvalósítani a getterek józan ellenőrzését az SDK-nkban, amely segít a potenciálisan rosszindulatú rendelések szűrésében.
[M02] Az ERC721 parancsok módosíthatók
Az ERC20-nál több cserére is van lehetőség OrderMixin
olyan szerződés telepítésével, amely ugyanazt a funkcióválasztót használja, mint az IERC20 transferFrom
, és a szerződést a makerAsset
vagy a takerAsset
sorrendben.
A hatókörön kívüli proxyk, nevezetesen ERC721Proxy
, ERC721ProxySafe
és ERC1155Proxy
szerződések ezt a mintát követik, hogy támogatást nyújtsanak ERC721
és a ERC1155
tokenek. Mivel a proxykat ugyanazzal a mintával kell meghívni, mint az IERC20-at transferFrom
hívást, az aláírásnak ezzel kell kezdődnie address from
, address to
és a uint256 amount
. Bármi más, amit a proxyk igényelnek, átadható utána, és a következő sorrendben határozható meg makerAssetData
és a takerAssetData
.
Az ERC1155-ök természetesen több azonos azonosító tokent is képesek egyszerre átvinni, ami azt jelenti, hogy a ERC1155Proxy
szerződés igénybe veszi a amount
terület. Másrészről, ERC721
s nincs nyilvánvaló haszna a amount
terület. Mivel nem helyettesíthető tokeneket képviselnek, egy adott tokenId csak egy létezik, ami a amount
mező haszontalan. Emiatt a megvalósítás mindkettőnél ERC721Proxy
és a ERC721ProxySafe
szerződések használja a szükséges amount
mező, mint a tokenId
helyette.
Ez a túlterhelés a amount
paraméter lehetőséget teremt a részleges kitöltésre ERC721
megrendelések külön listázott tokenek kedvezményes áron történő vásárlásához. Előfordulhat például olyan eset, amikor egyetlen felhasználónak több is van ERC721
s ugyanazon szerződés átruházását engedélyezte a ERC721Proxy
szerződést, és azokat külön limit megbízásokban sorolja fel.
Ha a limit megbízások is biztosítják a getMakerAmount
és a getTakerAmount
mezőket, lehetőség lesz ezek részleges kitöltésére ERC721
parancsokat. A rendelés óta amount
mező valójában megfelel a tokenId
, egy rosszindulatú felhasználó részleges kitöltést helyezhet el a ERC721
a magasabb tokenId-vel, ami a makingAmount
/takingAmount
Egy ERC721
ami egy alacsonyabbnak felelhet meg tokenId
. Az eredmény a ERC721
az alsóval tokenId
áron kerülne átadásra (higher tokenId price) * (lower tokenId's id) / (higher tokenId's id)
.
Ennek az exploitnak van néhány követelménye:
- Többszörös
ERC721
s ugyanabból a szerződésből, hogy engedjék bármelyikreERC721
egyetlen tulajdonos meghatalmazottja. - Nyitott rendelés az egyikre
ERC721
s ez nem a legalacsonyabbtokenId
az engedélyezettek közül. - Részleges kitöltések megengedettek a rendelésen.
A részleges lehetőség teljes megszüntetésére ERC721
kitölti, fontolja meg a amount
és a tokenId
érvek. Függetlenül attól, hogy az argumentumok külön vannak-e, vagy sem, fontolja meg ennek dokumentálását is, hogy figyelmeztesse a felhasználókat erre a viselkedésre, és elkerülje ezt a mintát a jövőben.
Frissítés: Rögzítve lehúzási kérés #59.
[M03] Nem dokumentált decimális feltételezések
A LimitOrderProtocol
szerződés örökli a ChainlinkCalculator
keresztül szerződést köt OrderMixin
szerződés. Ez a szerződés két olyan funkciót tesz lehetővé, amelyek lehetővé teszik a Chainlink orákulumok használatát a folyamat során állítmányok ellenőrzése és a keresés a készítői mennyiség/átvevő összeget.
A szerződés azonban dokumentálatlan feltételezéseket tartalmaz a tizedesjegyek számáról, amelyeket a Chainlink orákulumoknak kell jelenteniük, valamint azt, hogy a függvényparamétereknek hány tizedesjegyet kell tartalmazniuk. Bizonyos forgatókönyvek esetén ez váratlan viselkedésekhez vezethet, beleértve az eszközök hibás árazását és a pénzeszközök nem szándékos elvesztését.
Pontosabban, a szerződésben az az implicit feltételezés, hogy a Chainlink orákulumok 18 tizedesjegy pontossággal fognak jelentést készíteni. Azonban nem minden Chainlink jóslat jelentést ezzel a tizedesjegy számmal. Valójában, ha az orákulum egy pénznemben (például USD-ben) kifejezett tokenpárt jelent, akkor csak 8 tizedesjegy pontossága lesz. Mivel nincsenek korlátozások ami orákulumok használhatók, nem szabad implicit feltételezéseket tenni a jelentésükben szereplő tizedesjegyek számáról.
Ezzel kapcsolatban van egy implicit feltételezés, hogy a amount
paraméter a ChainlinkCalculator
függvények 18 tizedesjegyet fognak használni, valamint a félrevezető kifejezett nyilatkozatot, hogy a singlePrice
funkció Calculates price of token relative to ETH scaled by 1e18
. A valóságban még olyan orákulummal is nem jelentést 18 tizedesjegygel, a visszatérési értéke a singlePrice
függvény tizedesjegyeinek számával lenne skálázva amount
paramétert, amely nem feltétlenül 18 tizedesjegy.
Hasonlóképpen, a doublePrice
A függvény feltételezi, hogy két Chainlink orákulum ugyanannyi tizedesjegygel fog jelentést készíteni, ami miatt a függvény eredménye eltér a várttól.
Fontolja meg a feltételezések explicit dokumentálását a tizedesjegyek számával kapcsolatban, amelyet a paramétereknek és a visszatérési értékeknek tartalmazniuk kell. Ezenkívül fontolja meg vagy korlátozza a számításokat, amelyek olyan orákulumoktól függenek, amelyek megszegik ezeket a feltevéseket, vagy azt, hogy a vonatkozó számítások vegyék figyelembe a tizedesjegyek tényleges számát.
Frissítés: Rögzítve lehúzási kérés #75.
Alacsony súlyosság
[L01] A konstansok nincsenek kifejezetten deklarálva
A kódbázisban néhány esetben megmagyarázhatatlan jelentéssel bíró literális értékeket használnak. Például:
- A
OrderMixin
szerződés, a_remaining
A leképezés szemantikailag túlterhelt (a kérdésben leírtak szerint A leképezés szemantikai túlterhelése). szintén ha egy rendelést teljesen kitöltöttek. Kimondottan,0
azt jelenti, hogy nem történt megrendeléshez kapcsolódó kitöltés,1
azt jelenti, hogy egy rendelés már nem teljesíthető, és bármi nagyobb, mint1
azt jelenti, hogy van egy fennmaradó összeg a megrendeléshez, amely potenciálisan kitölthető. - A
ChainlinkCalculator
szerződés, a szó szerinti érték1e18
asinglePrice
funkciót.
A kód olvashatóságának javítása és az újrafeldolgozás megkönnyítése érdekében fontolja meg egy konstans meghatározását minden mágikus számhoz, és adjon neki egyértelmű és magától értetődő nevet. Összetett értékek esetén fontolja meg egy soron belüli megjegyzés hozzáadását, amely elmagyarázza, hogyan számították ki őket, vagy miért választották őket.
Frissítés: Rögzítve lehúzási kérés #75 és a lehúzási kérés #76.
[L02] A rosszindulatú felek megakadályozhatják a megengedett parancsok végrehajtását
A OrderMixin
szerződés lehetővé teszi a készítő felhasználók számára, hogy benyújtsák megengedett megrendelések így ezek egy tranzakcióban is végrehajthatók, nem pedig külön tranzakcióra lenne szükség a jóváhagyáshoz. A megrendelők is megtehetik benyújtják saját engedélyüket az azonos célú megrendelés kitöltése során.
Mivel azonban a gyártó engedélye benne van a érdekében, mind a gyártó, mind az átvevő engedélyei elérhetők lennének, amíg a rendelés-kitöltés tranzakció a mempoolban van. Ez lehetővé tenné bármely rosszindulatú felhasználó számára, hogy megszerezze ezeket az engedélyeket, és végrehajtsa azokat a megfelelő eszközszerződéseken a kitöltési tranzakció előfuttatása közben. Mivel ezek az engedélyek a nonce
A kettős kiadási támadás elkerülése érdekében a megbízás kitöltési tranzakciója meghiúsulna, ha ugyanazt az engedélyt próbálták meg használni, amelyet az előfutás során használtak.
Bár biztonsági kockázat nem áll fenn, és a gyártó létrehozhat egy új rendelést és előzetesen jóváhagyhatja a tranzakciót, ez a támadás minden bizonnyal hatással lehet az engedélyezett megrendelések használhatóságára. Valóban, egy motivált támadó blokkolhat minden megengedett parancsokat ezzel a támadással. Fontolja meg az érvényesítést, ha az engedélyt már benyújtották, vagy ha elegendő a pótlék, a megrendelés kitöltése során. Fontolja meg azt is, hogy a megrendelés összeállítása során tájékoztassa a felhasználókat erről a lehetséges támadásról.
Frissítés: Nincs kijavítva. Az 1 hüvelykes csapat kijelenti:
Korábban is végeztünk jóváhagyási ellenőrzéseket, de úgy döntöttünk, hogy egyszerűsítjük az engedélyezési folyamatot, hogy csak a sikertelen jóváhagyások esetén térjünk vissza. Megfontoljuk, hogyan értesíthetjük a gyártókat a problémáról.
[L03] Duplikált kód
A kódbázison belül előfordulnak ismétlődő kódok. A kód megkettőzése problémákhoz vezethet a fejlesztési életciklus későbbi szakaszában, és a projektet hajlamosabbá teszi a hibák bevezetésére. Ilyen hibák véletlenül léphetnek fel, ha a funkcionalitás változásait nem replikálják a kód minden olyan példányára, amelynek azonosnak kell lennie. Példák a duplikált kódra:
A kód megkettőzése helyett fontolja meg, hogy csak egy szerződést vagy könyvtárat tartalmazzon, amely tartalmazza a duplikált kódot, és használja azt bármikor, amikor a duplikált funkcióra van szükség.
Frissítés: Részben rögzítve lehúzási kérés #60.
[L04] Hibás vagy félrevezető tesztkészlet
A tesztcsomagban vannak olyan esetek, amikor a tesztek eltérnek a várt viselkedéstől. Például:
- A
ChainlinkCalculator
szerződést örökli aOrderMixin
szerződés. A tesztek során azonban aAmountCalculator.arbitraryStaticCall
függvény hívására szolgálChainlinkCalculator
szerződést külső, önálló szerződésként. Annak ellenére, hogy az eredmény a várt, a tesztnek tükröznie kell a viselkedést a rendszer jelenlegi felépítésével és a várható használati esettel aChainlinkCalculator
közvetlenül működik, tetszőleges statikus hívás nélkül. - Bár a meghatalmazotti szerződések nem terjedtek ki, észrevettük, hogy a protokoll ERC721 eszközökkel történő tesztelésekor a
ERC721Proxy
szerződést nem használják fel a benne lévő eszközök cseréjére tesztcsomag.
Mivel maga a tesztcsomag nem tartozik az ellenőrzés hatálya alá, kérjük, fontolja meg a tesztcsomag alapos áttekintését, hogy megbizonyosodjon arról, hogy minden teszt sikeresen fut-e a protokoll specifikációi szerint.
Frissítés: Rögzítve lehúzási kérés #57, lehúzási kérés #59és lehúzási kérés #61.
[L05] Hibák és kihagyások az eseményekben
A kódbázisban általában eseményeket bocsátanak ki, amikor érzékeny módosításokat hajtanak végre a szerződéseken. Sok esemény azonban nem tartalmaz indexelt paramétereket és/vagy hiányoznak fontos paraméterek. Például:
Vannak olyan kényes műveletek is, amelyekben nincsenek események, például:
Fontolja meg a meglévő események teljesebb indexelését, és új paraméterek hozzáadását ott, ahol ezek hiányoznak. Ezenkívül fontolja meg az összes esemény olyan teljes körű kibocsátását, hogy azokat felhasználhassák a szerződés állapotának helyreállítására a láncon kívüli szolgáltatásokkal.
Frissítés: Nincs kijavítva. Az 1 hüvelykes csapat azonban hozzáadott egy orderRemaining
paraméter a OrderCanceled
esemény be lehúzási kérés #62.
Az 1 hüvelykes csapat kijelenti:
Azt találtuk, hogy az adatoknak csak korlátozott részhalmazára van szükség a frontend igényeinek kielégítéséhez. Kiterjedt elemzés esetén az összes javasolt mező elérhető nyomkövetéssel. Mert
OrderRFQMixin
elvárjuk, hogy az árjegyzők saját kifinomult módszert alakítsanak ki a törölt rendelések nyomon követésére.
[L06] Tárolási változások az eseménykibocsátás során
A NonceManager
szerződés, amikor a NonceIncreased
esemény kerül kibocsátásra, az üzenet küldőjének nonce-je is megnő.
Több művelet egyidejű végrehajtása megnehezítheti a kódbázis gondolkodását, hajlamosabbá teheti a hibákat, és a műveletek figyelmen kívül hagyásához vagy félreértéséhez vezethet.
A kód általános szándékosságának, olvashatóságának és egyértelműségének javítása érdekében fontolja meg a nonce érték növelését az esemény kiadása előtt.
Frissítés: Rögzítve lehúzási kérés #63.
[L07] Az inkonzisztens dekódolási módszerek eltéréseket okozhatnak az eredményekben
Az összes bővíthetőség és rugalmasság támogatása érdekében a Limit Order Protocolnak rutinszerűen foglalkoznia kell a dinamikus bájtadatokkal és a külső szerződésekből származó tetszőleges visszatérési értékekkel. Ennek eredményeként a protokoll tartalmaz egy ArgumentsDecoder
könyvtárat, hogy hatékonyabban konvertálja a dinamikus bájtértékeket alapvető adattípusokká. Ezt a könyvtárat azonban nem kizárólagosan, és bizonyos esetekben használják abi.decode
helyett használják. Ezenkívül néhány szerződést használnak abi coder v1
míg mások használják abi coder v2
. Az előbbi jobban teljesít a ArgumentsDecoder
könyvtár, míg az utóbbi további ellenőrzéseket végez dekódoláskor.
A különböző dekódolási módszerek következetlen használata finom eltéréseket eredményezhet a kódbázis szándéka és tényleges viselkedése között.
Például a simulateCalls
funkció csak a ArgumentsDecoder.decodeBool
funkció. Ha a simulateCalls
A függvény segítségével ellenőrizzük azokat a hívásokat, amelyek egy megbízás predikátum részében történnének, akkor eredményei eltérhetnek attól, ami a predikátumfeltételek kiértékelésekor ténylegesen történik, mivel különböző dekódolási módszereket alkalmaznak.
Tehát például, ha egy predikátum külsőt alkot staticcall
valamilyen függvényre, amely visszaadja a uint256
érték nagyobb egynél a vártnál bool
, akkor a hívás visszaáll, mert a visszatérési érték az -val dekódolva abi coder v2
„s abi.decode
amely nem fogad el olyan értékeket, mint bool
. Ha azonban pontosan ugyanazt a hívást kezdeményezi simulateCalls
, akkor azt egyszerűen meg lesz jelölve true
, Mert decodeBool
minden nullánál nagyobb értéket úgy kezel true
.
Ahhoz, hogy a simulateCalls
függvény teljes mértékben tükrözi a tényleges predikátumhívások viselkedését, fontolja meg annak módosítását, hogy használja abi.decode
.
Frissítés: Rögzítve lehúzási kérés #82.
[L08] A bemeneti ellenőrzés hiánya
A fillOrderToWithPermit
és a fillOrderTo
funkciói a OrderMixin
szerződést, valamint a fillOrderRFQToWithPermit
és a fillOrderRFQTo
funkciói a OrderRFQMixin
szerződést, ne érvényesítse a target
cím paraméter.
Ez lehetővé teszi, hogy a felhasználó véletlenül átadja a nulla címet, és ennek eredményeként zárolja azokat az eszközöket, amelyeket a megrendelés teljesítése után át kell vennie.
Annak biztosítása érdekében, hogy a felhasználók véletlenül se zárolják pénzeszközeiket, fontolja meg annak ellenőrzését, hogy a target
cím nem egyenlő a nulla címmel az idézett függvényekben.
Frissítés: Rögzítve lehúzási kérés #78.
[L09] Alacsony egységteszt-lefedettség
Az egységteszt lefedettsége a teljes projektre vonatkozóan körülbelül 75%, néhány szerződés különösen alacsony lefedettséggel rendelkezik.
Figyelembe véve az egységtesztek fontosságát a kód érvényesítésében és a regressziók megelőzésében az új funkciók újrafaktorálása és fejlesztése során, javasoljuk, hogy az egységtesztek lefedettségét jelentősen, legalább 95%-ra növeljék, és olyan szélső eseteket is bevonjanak, amelyek még a valószínűtlen helyzeteket is lefedik.
Frissítés: Nincs kijavítva.
[L10] Félrevezető vagy hiányos belső dokumentáció
A kódbázis során néhány félrevezető és/vagy hiányos belső dokumentáció esetét azonosították, és ezeket ki kell javítani.
A következőkben a félrevezető belső dokumentáció példái láthatók:
- A
ChainlinkCalculator
szerződés, asinglePrice
függvény NatSpec@notice
címke azt mondja, hogyCalculates price of token relative to ETH scaled by 1e18
, de valójában ennek eredménye az érték ofamount
által méretezett tokenek1e18
, ahol előfordulhat, hogy az orákulum nem jelent ETH-t (például egy olyan pár esetében, amely nem tartalmazza az ETH-t). - A
OrderRFQMixin
szerződés, ainvalidatorForOrderRFQ
függvény NatSpec@return
címke félrevezető, mert előfordulhat, hogy az idézet nincs kitöltve a megfelelő érvénytelenítő bit beállításához. A rendelés törölhető is. - A vonalakon 147, 165és 188 of
OrderMixin.sol
, a NatSpec@param
a címkék nem nyelvtanilagosak. - Online 20 of
ERC1155Proxy.sol
, a@notice
címke kimondja, hogy a kiszámított hash a kivonatolás eredményefunc_733NCGU
funkciót, ahol annak lennie kellfunc_301JL5R
funkció helyett.
A következő példák a hiányos belső dokumentációra:
- Funkciók a
AmountCalculator
szerződés nem írja le egyik paramétert sem. - A
ChainlinkCalculator
szerződés, asinglePrice
és adoublePrice
A függvények nem írják le az összes paramétert. - A
ImmutableOwner
szerződés, a nyilvános változónak és módosítónak nincs NatSpec. - A
InteractiveNotificationReceiver
szerződés, anotifyFillOrder
függvény egyik paramétert sem írja le. - A
LimitOrderProtocol
szerződés, aDOMAIN_SEPARATOR
függvénynek nincs NatSpec. - Események és térképezések a
NonceManager
nincs NatSpec. - A
OrderRFQMixin
szerződés,cancelOrderRFQ*
függvények nem írják le a visszatérési értékeket. - A
OrderMixin
szerződés, több funkcióból hiányzik a teljes NatSpec. - Online 168 of
OrderMixin.sol
és online 71 ofOrderRFQMixin.sol
, hiányzik belőle a@dev
címke. - Funkciók a
PredicateHelper
szerződés nem írja le az összes paramétert.
A világos belső dokumentáció alapvető fontosságú a kódex szándékainak felvázolásához. A beépített dokumentáció és a megvalósítás közötti eltérések súlyos tévhitekhez vezethetnek a rendszer várható viselkedésével kapcsolatban. Fontolja meg ezeknek a hibáknak a kijavítását, hogy elkerülje a félreértést a fejlesztők, a felhasználók és az auditorok számára egyaránt.
Frissítés: Részben rögzített. A címzett félrevezető dokumentáció lehúzási kérés #75 és a lehúzási kérés #77.
Az 1 hüvelykes csapat kijelenti:
Javítottuk a félrevezető dokumentumokat. A dokumentumok elkészítésére később kerül sor.
[L11] Horgok használata esetén DoS-megrendelés lehetséges
A OrderMixin
A szerződés olyan funkcionalitást valósít meg, amely az általános láncon kívüli swap megbízásokat teljesíti, amelyeknek feltétele lehet a sikerük. A rendelés kitöltése során a rendelés lehet ellenőrizze az előre meghatározott „predikátum” feltételeket mielőtt folytatná a végrehajtást.
Mivel azonban ezek az előfeltételek bármely tetszőleges szerződés logikáját célozhatják, a rosszindulatú készítők elhitethetik az elfogadókkal, hogy a megrendelés megfelelően működik, és érvényes, amikor ellenőrzi azt a láncon kívül, de meghiúsul, amikor megpróbálja teljesíteni ugyanazt a rendelést. láncon belül. Ez a változás a predikátum viselkedésében történhet valamilyen változó állapot előfuttatásával, amelytől a predikátumok függnek, vagy megvizsgáljuk a küldött gázt, vagy akár azt is, hogy mely címek vesznek részt a hívásban, vagy valamilyen más logika segítségével.
Továbbá, ha a készítő meghatározta a interakció a csere során, a interactionTarget
szerződés visszaállíthatja magát, vagy visszavonhatja a kedvezményt, hogy megakadályozza a sikeres rendeléskitöltést, ami lényegében ugyanazt az eredményt eredményezi, mint a rosszindulatú predikátumok.
Bár az eszközök nem lesznek veszélyben, a kedvező megrendelést találó felhasználókra vagy robotokra nagyobb teher hárul majd, ha megpróbálják azonosítani az ilyen típusú kéretlen leveleket, amelyek a felszínen jogosnak tűnhetnek. Abban az esetben, ha nem azonosítják az ilyen típusú megrendeléseket, felesleges gázköltséget kell fizetniük. A kéretlen rendelések számának csökkentése érdekében fontolja meg az elérhető célpontok korlátozását ezeknél a hookoknál. Fontolja meg a felhasználók figyelmeztetését is erre a lehetőségre, mielőtt megpróbálják teljesíteni a rendeléseket.
Frissítés: Nincs kijavítva. Az 1 hüvelykes csapat kijelenti:
Ezt a háttérrendszerünkön kezeljük, és átgondoljuk, hogyan értesítsük a lehetséges elfogadókat a problémáról.
[L12] A kerekítés kedvezőtlen lehet taker
A OrderMixin
és a OrderRFQMixin
szerződések, amikor egy rendelést teljesítenek, és az átvevő csak a makingAmount
or takingAmount
összeg, a protokoll megkísérli kiszámítani a swap ellenértékét.
Ezekkel a számításokkal két probléma van, az első az, hogy nincs olyan dokumentáció vagy logika, amely korlátozza az összeg paramétereinek használandó tizedesjegyek számát, amit a Dokumentálatlan decimális feltevések probléma.
A második kérdés az, hogy e számítások során a jegyzőkönyv a készítő javára kerekedik. A kerekítési probléma nagymértékben súlyosbodhat, ha az implicit decimális feltevések megsérülnek, de még akkor is, ha minden a várt módon történik, a kerekítés kis, páratlan összegekkel történik.
Fontolja meg annak engedélyezését, hogy az elfogadó megadja a minimális mennyiséget makerAsset
vagyontárgyat, amelyet hajlandóak átvenni a maximális összeggel együtt takerAsset
hajlandóak cserélni, így minden kerekítés elfogadása egyértelműbb.
Frissítés: Nincs kijavítva. Az 1 hüvelykes csapat kijelenti:
A küszöbértéknek elegendőnek kell lennie az átvevő védelméhez.
[L13] Ellentmondásos rendeléskezelés paraméterek hiányában
A OrderMixin
szerződés, a fillOrderTo
funkció belső hívásokat indít a _callGetMakerAmount
és a _callGetTakerAmount
funkciókat, amikor megpróbálja kitölteni, és vagy a makingAmount
vagy a takingAmount
paraméterei rendre nullák, vagy ha a makingAmount
érték nagyobb, mint a remainingMakerAmount
értéket.
A _callGetMakerAmount
és a _callGetTakerAmount
hívások visszaállításhoz vezetnek, ha a rendelés nem a getMakerAmount
or getTakerAmount
paramétereket, és részleges kitöltést hajtanak végre.
An szövegközi megjegyzés mellett _callGetMakerAmount
és egy szövegközi megjegyzés mellett _callGetTakerAmount
azt állítják, hogy „csak egész kitöltések megengedettek”, ha a rendelés nem ezzel jött létre getMakerAmount
or getTakerAmount
paramétereket.
Vannak azonban olyan kódútvonalak, amelyekre ez nem vonatkozik, mert ezek az útvonalak nem ellenőrzik a length
s mindkettőből getMakerAmount
és a getTakerAmount
paramétereket.
Kifejezetten, amikor a taker
meghatározza a takerAmount
érték olyan rendelés esetén, amely csak a getMakerAmount
, hacsak nem hívja getMakerAmount
ennél nagyobb összeget ad vissza remainingMakerAmount
, a beépített dokumentációval ellentétben részleges kitöltés is végrehajtható.
Ez tisztázatlanná teszi ezen kódútvonalak szándékosságát. Ha ez a várt viselkedés, fontolja meg a szövegközi dokumentáció módosítását, hogy az egyértelműbb legyen. Ha ez nem szándékos viselkedés, mindig ellenőrizze mindkét hosszát getMakerAmount
és a getTakerAmount
paramétereket egyszerre, így a megvalósítás megerősíti a beépített dokumentációban leírt viselkedést.
Frissítés: Rögzítve lehúzási kérés #79.
[L14] Elavult Chainlink hívások használata
A ChainlinkCalculator
szerződés célja a Chainlink orákulumok lekérdezése. Ezt úgy teszi, hogy felhívja őket latestTimestamp
és a latestAnswer
mód, mindkettő elavult. Valójában a módszerek már nincsenek jelen a Chainlink aggregátorok API-jában a harmadik verziótól kezdve.
A Chainlink orákulumokkal való esetleges jövőbeni összeférhetetlenség elkerülése érdekében fontolja meg a latestRoundData
módszer helyett.
Frissítés: Rögzítve lehúzási kérés #67.
Megjegyzések és további információk
[N01] Nem importál interfészt
A AggregatorInterface
Az interfész a kód egy részhalmazának tűnik, amelyről másolt ChainLink
nyilvános kódtárában. A teljes felületet tartalmazza ChainLink
szerződéses npm csomagja.
Ha lehetséges, az interfész eltérések és az ebből eredő problémák csökkentése érdekében egy másik projekt interfészeinek újradefiniálása és/vagy átírása helyett fontolja meg a hivatalos npm-csomagokon keresztül telepített interfészek használatát.
Frissítés: Rögzítve lehúzási kérés #66.
[N02] Elavult projektfüggőségek
A telepítés során a a projekt függőségei, az NPM arra figyelmeztet, hogy az egyik telepített csomag, Highlight
, „a jövőben nem lesz támogatott, és nem kap biztonsági frissítéseket”.
Annak ellenére, hogy nem valószínű, hogy ez a csomag biztonsági kockázatot jelentene, fontolja meg a csomagot használó függőség frissítését egy karbantartott verzióra.
Frissítés: Rögzítve lehúzási kérés #64.
[N03] A metódusok megtekintésére irányuló külső hívások nem statikus hívások
A kódbázis nagy részében a protokoll kifejezetten külső hívásokat bonyolít az OpenZeppelin keresztül. functionStaticCall
módszer az állapotváltozások lehetőségének korlátozására, ha azok nem várhatóak vagy nem kívánatosak. Azonban a ChainlinkCalculator
szerződést, annak ellenére, hogy csak külső hívást kíván kezdeményezni view
módszerek a Chainlink orákulumokon, a külső hívások a singlePrice
és a doublePrice
A funkciók nem explicit módon jönnek létre staticcall
s.
Bár nem azonosítottunk ebből fakadó azonnali biztonsági aggályokat, a támadási felület csökkentése, a következetesség javítása és a szándék tisztázása érdekében fontolja meg az explicit staticcall
s, minden külső hívás esetén view
funkciók a ChainlinkCalculator
szerződés.
Frissítés: Nincs kijavítva. Az 1 hüvelykes csapat kijelenti:
Úgy gondoljuk, hogy a szintaxis bonyolultsága semmissé teszi a konzisztencia javítását.
[N04] Korai kudarc érvénytelen rendelésekkel
A OrderMixin
szerződés, a fillOrderTo
funkció kezeli a speciális feltételt, ha korábban nem adtak fel rendelést (remainingMakerAmount == 0
), de nem kezeli kifejezetten azt a feltételt, amikor a megrendelés már nem érvényes (remainingMakerAmount == 1
).
Az utóbbi forgatókönyvben a funkció végül visszaáll, de csak nem triviális mennyiségű gáz elégetése után. A szándék tisztázása, az olvashatóság növelése és a gázfogyasztás csökkentése érdekében fontolja meg az érvénytelen rendelés forgatókönyvének kifejezetten a funkció kezdete felé történő kezelését.
Frissítés: Rögzítve lehúzási kérés #68.
[N05] A segítői szerződések nincsenek megjelölve elvontként
A Solidityben a kulcsszó abstract
olyan szerződéseknél használatos, amelyek vagy nem funkcionális szerződések önmagukban, vagy nem ekként való használatra készültek. Helyette, abstract
a szerződéseket a rendszer más szerződései öröklik, hogy önálló funkcionális szerződéseket hozzanak létre.
Az egész kódbázisban vannak példák olyan segítői szerződésekre, amelyek nincsenek megjelölve elvontként, annak ellenére, hogy nem önálló telepítésre készültek. Például a AmountCalculator
, ChainlinkCalculator
, ImmutableOwner
, NonceManager
és PredicateHelper
A szerződések mindegyike olyan alapfunkciókból áll, amelyeket öröklési szerződések szánnak.
Fontolja meg a segítői szerződések megjelölését abstract
egyértelműen jelezni, hogy kizárólag az őket öröklő szerződések funkcionalitásának bővítésére szolgálnak.
Frissítés: Nincs kijavítva. Az 1 hüvelykes csapat kijelenti:
Ezek a segítők külön is bevethetők. Csak a gázmegtakarítás miatt öröklődnek.
[N06] Inkonzisztens függvénysorrend
A kódbázisban a függvények sorrendje általában követi a javasolt sorrend a Solidity Style Guide-ban, ami: constructor
, fallback
, external
, public
, internal
, private
.
Azonban a OrderMixin
szerződés, a public
checkPredicate
függvény eltér a stíluskalauztól, felezve a external
funkciókat.
A projekt általános olvashatóságának javítása érdekében fontolja meg a függvények sorrendjének szabványosítását a kódbázisban, a Solidity Style Guide ajánlása szerint.
Frissítés: Rögzítve lehúzási kérés #69.
[N07] Inkonzisztens rendeléskitöltési folyamat
A OrderMixin
és a RFQOrderMixin
A szerződések egyaránt kezelik az aláírt megrendelések teljesítését, de a két szerződés közötti általános rendelési folyamat inkonzisztens.
OrderMixin
„s fillOrderTo
függvény ezt az általános folyamatot követi (pszeudokód):
if ((takingAmount == 0) == (makingAmount == 0))
else if (takingAmount == 0)
else (handle makingAmount == 0) THEN swapTokens
mivel RFQOrderMixin
analóg fillOrderRFQTo
függvény ezt a folyamatot követi (pszeudokód):
if (takingAmount == 0 && makingAmount == 0)
else if (takingAmount == 0)
else if (makingAmount == 0)
else revert THEN swapTokens
A dokumentációból nem derül ki, hogy miért különbözik az első feltétel e két függvényben, vagy miért takingAmount
és a makingAmount
az utóbbi függvényben mindkettő nem lehet nulla. Az az eset is, amikor mind a makingAmount
és egy takingAmount
vannak biztosítva sokkal könnyebb érvelni a fillOrderRFQTo
funkciót, mivel a döntőben egyértelműen kezelik else
Blokk.
A szándék tisztázása és a kód általános olvashatóságának növelése érdekében fontolja meg a két szerződés általános rendelési folyamatának szabványosítását, vagy kifejezetten dokumentálja a különbségek okát.
Frissítés: Nincs kijavítva. Az 1 hüvelykes csapat kijelenti:
Ez a limitált megbízások egyedi árazási funkcióinak köszönhető. Mivel
getMakerAmount
jelentősen eltérhet ettőlgetTakerAmount
, úgy gondoltuk, hogy jobb, ha nem adjuk meg az alapértelmezett beállítást az átvevő számára, mert valószínűleg összezavarja őket abban az esetben, ha ezek a getterek eltérőek lesznek.
[N08] A hibaüzenetek formátuma nem következetes vagy félrevezető
Az egész kódbázisban a require
és a revert
A hibaüzenetek, amelyek célja, hogy értesítsék a felhasználókat a tranzakció sikertelenségét okozó konkrét körülményekről, nem következetesen formáztak.
Például minden egyes hibaüzenet a vonalakon 85 of OrderMixin.sol
, 16 of ERC721ProxySafe.sol
és 26 of Permitable.sol
más stílust alkalmaznak.
Ezenkívül néhány hibaüzenet félrevezető:
A hibaüzenetek célja, hogy értesítsék a felhasználókat a meghibásodásokról, ezért elegendő információt kell adniuk ahhoz, hogy a megfelelő korrekciókat el lehessen végezni a rendszerrel való interakcióhoz. A nem informatív hibaüzenetek nagymértékben rontják az általános felhasználói élményt, így rontják a rendszer minőségét. Ezenkívül a nem következetesen formázott hibaüzenetek szükségtelen zűrzavart okozhatnak. Ezért fontolja meg a teljes kódbázis áttekintését, hogy megbizonyosodjon arról, hogy minden require
és a revert
Az utasítás következetesen formázott, pontos, informatív és felhasználóbarát hibaüzenetet tartalmaz.
Frissítés: Részben rögzítve lehúzási kérés #81.
[N09] Elnevezett visszatérési változók következetlen használata
Az elnevezett visszatérési változók következetlen használata a OrderMixin
szerződés. Néhány funkció elnevezett változókat ad vissza, mások explicit értékeket ad vissza, és mások megnevezett visszatérési változót deklarál, de felülírja explicit return utasítással.
Fontolja meg a visszatérési értékek következetes megközelítését a kódbázisban az összes megnevezett visszatérési változó eltávolításával, kifejezetten helyi változóként való deklarálásával, és adott esetben a szükséges visszatérési utasítások hozzáadásával. Ez javítaná a kód egyértelműségét és olvashatóságát, és segíthet csökkenteni a regressziókat a jövőbeni kódrefaktorok során.
Frissítés: Rögzítve lehúzási kérés #73.
[N10] A rendelés hash számítása nem nyitott az API számára
A external
funkciók remaining
, remainingRaw
és a remainingsRaw
mindegyik megrendelési hash-t vár a sikeres működéshez.
Azonban a segítő funkció _hash
, amely egy rendelés hash-jét adja vissza, rendelkezik private
láthatóság. Ez azt jelenti, hogy a felhasználóknak manuálisan kell csomagolniuk a rendelések és a tartományi karakterláncok egyes részeit, hogy megkapják a rendelés kivonatát.
Annak érdekében, hogy elkerülje a hibákat a rendelési hash kiszámításakor, és hogy a felhasználók számára módszert biztosítson a rendelés megfelelő kivonatának generálására, fontolja meg a _hash
funkció public
és a név átalakítása hash
hogy összhangban legyen a kód többi részével.
Frissítés: Rögzítve lehúzási kérés #74.
[N11] A leképezés szemantikai túlterhelése
A _remaining
feltérképezése a OrderMixin
A szerződés szemantikailag túlterhelt, hogy nyomon követhesse a megrendelések állapotát és a rendelésekhez rendelkezésre álló eszközök fennmaradó mennyiségét.
A három állapot, amelyet felvehet:
0
: A rendelési hash még nem volt látható.1
: A rendelést törölték vagy teljesen kitöltötték.- Bármilyen
uint
nagyobb, mint1
: A maradékmakerAmount
a rendelés pluszban tölthető ki1
.
Ez a szemantikai túlterhelés megköveteli ennek az értéknek a tördelését és kicsomagolását közben lookup
, cancellation
, initialization
és storage
.
A szemantikai túlterhelés és az azt lehetővé tévő szükséges logika hajlamos lehet a hibákra, és megnehezítheti a kódbázis megértését és megfontolását, valamint a kód jövőbeli frissítései során regressziót is nyithat.
A kód olvashatóságának javítása érdekében fontolja meg a megrendelések teljesítési állapotának külön leképezésben történő nyomon követését.
Frissítés: Nincs kijavítva. Az 1 hüvelykes csapat arra hivatkozott, hogy a javítás növelné a gázköltséget fillOrder
funkciót.
[N12] Az engedéllyel rendelkező megrendelések önkényes szerződések lehívását teszik lehetővé
A OrderMixin
szerződés örökli a Permitable
szerződést, amely lehetővé teszi az egyszeri tranzakciós megbízás kitöltését olyan eszközökkel, amelyek elfogadják ezeket permit
pótlékok módosítására szólít fel.
Azonban a felhívja a Permitable
szerződés ne ellenőrizze, hogy a cél megengedett eszköz-e, és még azt sem, hogy a rosszindulatú felhasználó átadja egy tetszőleges szerződés címét, amely újabb hívást hajthat végre, mielőtt a megrendelés kitöltése befejeződik.
Bár a szerződés az visszalépés ellen védett, mindig ajánlott a támadási felület csökkentése és a külső szerződések lehívásának megakadályozása a végrehajtás során. Fontolja meg az engedélyben szereplő tartalom korlátozását a megrendelésben érintett eszközökre, vagy a protokoll eszközeinek engedélyezőlistájára.
Frissítés: Nincs kijavítva. Az 1 hüvelykes csapat kijelenti:
OrderMixin
valójában nem rendelkezik információval a tényleges tokenekről, mintmakerAsset
és atakerAsset
néha proxy-k vagy egyéb köztes szerződések, és a tényleges tokenekkel kapcsolatos információk tetszőleges bájtokban tárolódnak. Tehát nincs járható mód annak korlátozására, hogy melyik eszköztpermit
hívják.
[N13] solhint
soha nem engedélyezi újra
A kódbázisban van néhány solhint-disable
nyilatkozatok, különösen az online 23 és online 41 of RevertReasonParser.sol
, amelyek nem záródnak le solhint-enable
.
Az explicititás mellett, és a lehető legkorlátozóbbnak kell lenni a letiltáskor solhint
, fontolja meg a használatát solhint-disable-line
or solhint-disable-next-line
ehelyett vonalhoz hasonlóan 16 ugyanabból a fájlból.
Frissítés: Rögzítve lehúzási kérés #72.
[N14] Elírási hibák
A kódbázis a következő elírási hibákat tartalmazza:
Ráadásul a projekt README
(a jelen ellenőrzésen kívül) a következő elírásokat tartalmazza:
Fontolja meg ezen elírási hibák kijavítását a kód olvashatóságának javítása érdekében.
Frissítés: Rögzítve lehúzási kérés #71 és a lehúzási kérés #77.
[N15] Használata uint
helyett uint256
Az explicititás előnyben részesítése érdekében a uint
ként kell bejelenteni uint256
. Különösen azok, amelyek a for
hurkok a vonalakon 98 és a 119 of OrderMixin.sol
és vonalak 16 és a 30 of PredicateHelper.sol
.
Frissítés: Rögzítve lehúzási kérés #70.
Következtetések
3 súlyos problémát találtak. Néhány változtatást javasoltak a bevált gyakorlatok követése és a lehetséges támadási felület csökkentése érdekében.
- &
- 7
- Rólunk
- hozzáférés
- Szerint
- Fiók
- át
- törvény
- cselekvések
- További
- cím
- Előny
- Minden termék
- lehetővé téve
- már
- Bár
- Összegek
- elemzés
- api
- megközelítés
- érvek
- körül
- vagyontárgy
- Eszközök
- könyvvizsgálat
- Back-end
- Kezdet
- hogy
- BEST
- legjobb gyakorlatok
- Bit
- botok
- épít
- hívás
- ami
- esetek
- Okoz
- Láncszem
- változik
- ellenőrzése
- Ellenőrzések
- kód
- bonyolult
- feltétel
- zavar
- építés
- tartalmaz
- szerződés
- szerződések
- Hiba
- kiadások
- tudott
- Pár
- Teremtő
- Valuta
- Jelenlegi
- dátum
- üzlet
- Denial of Service
- bevezetéséhez
- Design
- fejlesztők
- Fejlesztés
- DID
- különbözik
- különböző
- domain
- kétszeresére
- dinamikus
- Korai
- él
- ösztönzése
- különösen
- ETH
- esemény
- események
- minden
- példa
- csere
- várható
- tapasztalat
- Exploit
- Jellemzők
- Fields
- Végül
- vezetéknév
- Rögzít
- Rugalmasság
- áramlási
- következik
- talált
- Tele
- funkció
- alapok
- jövő
- játék
- GAS
- általános
- Giving
- nagy
- útmutató
- Kezelés
- hash
- tördelő
- tekintettel
- segít
- Magas
- nagyon
- Hogyan
- HTTPS
- hibrid
- azonosítani
- Hatás
- végre
- fontos
- importáló
- beleértve
- Beleértve
- Növelje
- <p></p>
- info
- információ
- Infrastruktúra
- meglátások
- A szándék
- kamat
- Felület
- részt
- kérdések
- IT
- nagy
- nagyobb
- vezet
- vezető
- könyvtár
- Korlátozott
- vonal
- fizetőképesség
- Listázott
- listák
- helyi
- nézett
- lookup
- fontos
- készítő
- Gyártás
- piacára
- Mempool
- tükör
- modell
- a legtöbb
- Legnepszerubb
- ugyanis
- Új funkciók
- nem helyettesíthető
- nem helyettesíthető tokenek
- hivatalos
- nyitva
- Művelet
- opció
- jóslat
- érdekében
- rendelés
- Más
- tulajdonos
- Mintás
- Népszerű
- be
- megakadályozása
- ár
- árazás
- magán
- folyamat
- program
- projektek
- védelem
- protokoll
- ad
- biztosít
- meghatalmazott
- nyilvános
- közzétesz
- Vásárlás
- világítás
- emel
- Valóság
- csökkenteni
- bizalom
- jelentést
- Jelentések
- raktár
- követelmények
- REST
- Eredmények
- Visszatér
- Kritika
- Kockázat
- fordulóban
- futás
- sdk
- biztonság
- Szolgáltatások
- készlet
- megosztott
- Megoszt
- váltás
- hasonló
- Egyszerű
- kicsi
- okos
- Intelligens szerződések
- So
- szilárdság
- spam
- kifejezetten
- Költési
- Spot
- kezdet
- Állami
- nyilatkozat
- Államok
- Állapot
- tárolás
- stílus
- benyújtott
- siker
- sikeres
- sikeresen
- támogatás
- Támogatott
- felületi
- kapcsoló
- rendszer
- cél
- teszt
- Tesztelés
- tesztek
- Keresztül
- egész
- idő
- együtt
- jelképes
- tokenek
- vágány
- Csomagkövetés
- tranzakció
- Bízzon
- egyedi
- Frissítés
- us
- használhatóság
- USAdollár
- USDT
- Felhasználók
- hasznosság
- érték
- Megnézem
- láthatóság
- várjon
- Mit
- engedélyező lista
- belül
- nélkül
- Munka
- érdemes
- nulla