16 Aralık 2021
Giriş
The 1inch ekibimiz bizden incelememizi ve denetlememizi istedi Limit Emir Protokolü v2 akıllı sözleşmeler. Koda baktık ve şimdi sonuçlarımızı yayınlıyoruz.
kapsam
Taahhüdü denetledik 4d94eea25e4dac6271bfd703096a5c4a4d899b4a
arasında 1inch/limit-order-protocol
depo. Kapsam dahilinde aşağıdaki sözleşmeler vardı:
- 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
Diğer tüm proje dosyaları ve dizinlerinin (testler dahil) yanı sıra dış bağımlılıklar ve projeler, oyun teorisi ve teşvik tasarımı da bu denetimin kapsamı dışında tutuldu. Harici kod ve sözleşme bağımlılıklarının belgelendiği gibi çalıştığı ve 1inch tarafından sağlanan arka uç hizmetlerinin protokolün çıkarına en uygun şekilde hareket ettiği varsayılmıştır.
Genel Sağlık
Genel olarak projenin kod tabanının okunabilir ve iyi organize edilmiş olduğunu gördük, ancak özellikle montaj kodu blokları, protokolün son durumları, kullanılacak varlıklar/yüklemler/harici kaynaklar gibi daha kapsamlı belgelerden faydalanabilir. sağlanan arka uç hizmetinin sorumlulukları/sınırlamaları ve aktörler arasındaki etkileşimler. Proje, eylemleri gaz açısından verimli hale getirmek için büyük çaba harcıyor, hatta bazen kodun akıl yürütmesini zorlaştırma riskini bile göze alıyor; bununla ilgili konuları aşağıda dile getiriyoruz. Denetim boyunca 1inch ekibi son derece ulaşılabilir, hızlı yanıt veren ve birlikte çalışması çok kolay bir ekipti.
sistem görünümü
Limit Emri Protokolü emri mümkün kılar makers
token takasları için zincir dışı siparişleri imzalamak. Protokol daha sonra önceden imzalanmış siparişlerin siparişe göre doldurulmasını kolaylaştırır takers
. Siparişler oldukça genişletilebilir ve sipariş doldurma süreci boyunca çeşitli noktalarda harici sözleşmeleri statik olarak çağırabilir. Bu genişletilebilirlik, protokole fayda sağlar, ancak siparişlerin kendisi için hem karmaşıklık hem de daha büyük bir saldırı yüzeyi ekler.
Sipariş ayrıntılarının zincir üzerinde saklanmadığını belirtmek önemlidir. Siparişlerin doluluk durumu veya iptal durumu yalnızca sipariş karmaları aracılığıyla izlenir. Bu, siparişlerin eşler arası veya merkezi bir taraf aracılığıyla paylaşılmasını gerektirir. Bu durumda 1inch ekibi, imzalanan emirleri toplayarak ve bu emirleri diğer protokolleri için bir likidite kaynağı olarak kullanarak merkezi taraf olarak hareket etmeyi planlıyor. Siparişler kendi API'leri aracılığıyla yayınlanacak, böylece kullanıcılar onlarla etkileşime geçebilecek.
Bu merkezileştirme, 1inch ekibine hangi siparişlerin yayınlanacağı ve nihai olarak yürütüleceği konusunda olağanüstü kontrol sağlar. Bu aynı zamanda onlara, kötü niyetli veya aldatıcı siparişler durumunda yararlı olabilecek, ancak aynı zamanda kötüye kullanılabilecek siparişleri sansürleme olanağı da verir ve olumlu bir sipariş durumunda bunu API aracılığıyla göstermeyerek başka herhangi bir kullanıcıyı önden yürütmelerine olanak tanır.
Ayrıcalıklı Roller
Rolün kullanıldığı sözleşmeler kapsam dışı olmasına rağmen ayrıcalıklı bir rol belirlendi. Bir immutableOwner
Oluşturma sırasında bir vekil sözleşmesinin yaratıcısına ayarlanır ve vekilin sözleşmelerine erişimi sınırlamak için kullanılır. external
fonksiyonlar.
Dış bağımlılıklar ve güven varsayımları
Bu protokolün tasarımı, zincir dışı ve zincir üstü bileşenleri gerektirir ve bu hibrit model, raporumuzda tanımladığımız bazı saldırı vektörlerini azaltmak için kullanılabilir, ancak bu yeteneğin maliyeti, 1 inçlik ekibe ve altyapıya olan bağımlılığın artmasıdır.
Ek olarak Limit Emri Protokolü, Chainlink oracle'larından fiyatları almayı amaçlayan işlevler sağlar. Bu kehanetlerin dürüst, erişilebilir ve düzgün çalıştığını varsaydık.
Ayrıca, bir siparişin esnekliği nedeniyle, dış sözleşmelerle doğrulanmayan çeşitli temas noktaları bulunmaktadır. Bu, kötü niyetli bir kullanıcının bu tür çağrıları kötüye kullanabileceği ve emirlerin yerine getirilmesi sırasında eylemleri gerçekleştirmek için kötü niyetli sözleşmelerle tahminlerin, varlıkların veya oracle'ların kimliğine bürünebileceği anlamına gelir. Proje bazı bölgelerde yeniden girişe karşı korunuyor olsa da, bu tür vektörler hizmet reddi saldırılarına veya tespit edilemeyen spam siparişlerine neden olabilir. 1inch ekibi, protokol için alışılmadık sözleşmeler kullanıldığında bazı sorunların ortaya çıkabileceğinin farkında ve proje tarafından yalnızca büyük "mavi çip" varlıkların tam olarak desteklenmesi niyetinde olduklarını belirtti. Bununla birlikte, en popüler varlıklarda bile, her bir varlığın, USDT ile transferler sırasında ücret alınması veya hata kodu yerine hata kodu döndürülmesi gibi, bunları doğru şekilde ele almayan protokollerde sorunlara neden olabilecek kendine özgü davranışların bulunduğu unutulmamalıdır. cTokens ile başarı boole değeri.
Bulgular
Burada bulgularımızı sunuyoruz.
Kritik şiddet
Yok.
Yüksek şiddet
[H01] Tutarsız veri aktarıldı _makeCall
içinde OrderMixin
sözleşme _makeCall
fonksiyon varlıkları aktarmak için kullanılır alıcıdan yapıcıya ve sonra yapıcıdan alıcıya. Son transferde, _makeCall
fonksiyon siparişin hatalı bir şekilde iletilmesi makerAsset
son parametre olarak, siparişin ne zaman olması gerektiği makerAssetData
.
Sonuç olarak, ona dayanan herhangi bir proxy işlevi makerAssetData
tartışma bozulacak.
Daha önceki çağrıyla tutarlı olmak için _makeCall
ve proxy işlevselliğini tam olarak desteklemek için, order.makerAsset
parametresi order.makerAssetData
.
Güncelleme: Sabit çekme isteği #57.
[H02] Kısmen doldurulmuş özel siparişler herkes tarafından doldurulabilir
Protokol, özel ve kamu düzenlerinin oluşturulmasına izin veriyor. Özel siparişlerde sadece allowedSender
Siparişin oluşturulması sırasında üretici tarafından belirtilen adres, siparişi yerine getirebilir.
Ancak, içinde OrderMixin
sözleşme için doğrulama allowedSender
adres kapsamı yanlıştır; bu, yalnızca bir siparişin ilk doldurulmasını işleyen mantık içinde değerlendirildiği anlamına gelir. Özel bir emrin kısmen doldurulması durumunda, allowedSender
adrese artık ulaşılamıyor ve sipariş herkes tarafından doldurulabilir hale geliyor.
Herhangi bir kullanıcının kısmen doldurulmuş özel siparişleri doldurup dolduramayacağına ilişkin amacı açıklığa kavuşturmak için mevcut davranışın nedenini belgelemeyi veya allowedSender
Her doldurma girişiminde doğrulanacağından emin olmak için adresi ilk doldurmanın kapsamı dışında tutun.
Güncelleme: Sabit çekme isteği #58.
[H03] Kötü niyetli yapıcı, alıcının varlıklarını çalmak için kısmi doldurmalardan yararlanabilir
Siparişler OrderMixin
sözleşme kısmen doldurulabilme özelliğine sahiptir. Kısmi doldurmaları desteklemek için protokol, takasların her iki tarafını da hesaplamanın bir yolunu gerektirir. İkisi birden getMakerAmount
ve getTakerAmount
alanlar, siparişi veren kişi tarafından tam olarak bu amaç için tanımlanır.
Bir siparişi doldururken, alıcılar aşağıdakilerden birini sağlamalıdır: makingAmount
ya da takingAmount
değerlerin yanı sıra bir thresholdAmount
değer. Aşağıdaki durumlara bağlı olarak alınabilecek iki farklı kod yolu vardır: makingAmount
ya da takingAmount
sağlandı.
Bunlardan ilki, makingAmount
parametre tanımlanır. O olabilir kesmek the makingAmount
değer ve aynı zamanda hesapla takingAmount
bunun için değer. Bu durumda, thresholdAmount
sağlar takingAmount
alınan değer beklenmedik derecede büyük değil.
İkincisi ise, takingAmount
parametre tanımlanır. Böyle bir durumda hesapla makingAmount
değer, olasılığı ile onu kısaltıyorum ve yeniden hesaplamak takingAmount
eğer bu gerçekleşirse değer. Bu durumda, thresholdAmount
değer şunu sağlar: makingAmount
döndürülen değer beklenmedik derecede küçük değil.
Her biri daha önce bahsedilen kod yollarından birine özgü olan iki yararlanma yöntemi mevcuttur. Bu istismar yöntemleri kötü amaçlı yazılım gerektirir getMakerAmount
ve getTakerAmount
işlevler. Bu işlevlerin basit bir uygulaması, aşağıdakilerle aynı davranışa sahip olacaktır: AmountCalculator
'ler getMakerAmount
ve getTakerAmount
ancak gerektiğinde saldırgan tarafından kontrol edilen bir değer döndürmeye zorlayacak sabit kodlu bir anahtarla.
Daha az şiddetli olan ilk yararlanma modeli, ilk kod yolunu içerir; makingAmount
değer belirtildi doldurma sırasına göre. Kötü niyetli bir yaratıcı, aşağıdakileri belirten bir doldurma emrini bekler: makingAmount
önden koşmak için bellek havuzunda görünmek. Yapımcı tarafından 1 dışındaki tüm değeri çekerler ve sonra zorlarlardı. _callGetTakerAmount
Kullanıcının belirttiği tutarı döndürmek için thresholdAmount
değeri (veya daha azsa ödenekleri). Kullanıcının işlemi nihayet tamamlandığında, tüm paralarını değiştirecekler. thresholdAmount
değerinde takerAsset
tek bir birim için makerAsset
. Bu yararlanma, tarafından verilen miktarla sınırlıdır. thresholdAmount
değeri veya miktarı takerAsset
kullanıcıya izin verildi LimitOrderProtocol
sözleşme.
İkinci, daha şiddetli istismar modeli, ikinci kod yolunu içerir; takingAmount
değer belirtildi. Kötü niyetli yaratıcı da benzer şekilde, belirtilen bir doldurma emrini bekleyecektir. takingAmount
Bellek havuzunda görünecek değer. İşlemi önden yürütecekler ve zorlayacaklardı. makingAmount
tarafından döndürülen değer _callGetMakerAmount
fonksiyonun her ikisinden de daha yüksek olması remainingMakerAmount
ve thresholdAmount
. Ayrıca şu ayarı da yapacaklardı: takingAmount
tarafından döndürülen değer _callGetTakerAmount
miktarı olmak takerAsset
üzerinde izin verilen varlık LimitOrderProtocol
alıcı tarafından. Alıcının işlemi gerçekleştiğinde, kısaltmak makingAmount
değer ve sonra yeniden hesapla takingAmount
değer. Ancak bu yeniden hesaplamanın daha düşük olacağı garanti edilmez ve bu durumda alıcının tüm takerAsset
sözleşmede buna izin verdiler. Bu kod yolunda, thresholdAmount
değer şudur emin olmak makingAmount
çok düşük değil, yani alıcının tüm haklarını alıyorum takerAsset
varlık işaretlenmemiş. Kaybedilen fonlar, takerAsset
kullanıcının izin verdiği varlık LimitOrderProtocol
sözleşme.
Bu istismarlar, kısmi siparişler ve daha spesifik olarak kötü niyetli kısmi siparişler olmadan mümkün değildir. getMakerAmount
ve getTakerAmount
uygulamalar.
Asıl mesele thresholdAmount
değer kontrolü, takasın yalnızca bir tarafını kapsamasıdır, ancak diğer taraf önden çalıştırma yoluyla manipüle edilebilir. Alıcının başlangıçta teklif ettiği değerin değişmeden kalacağına dair hiçbir güvence yoktur. Kaldırmayı düşünün makingAmount
her iki kod yolundan da kesme ve siparişin istendiği kadar büyük bir dolguyu destekleyememesi durumunda geri dönme. Bunu yaparak, thresholdAmount
takasın diğer tarafını yeterince kısıtlamak ve kötü niyetli emirlerde bile beklenmeyen davranışları önlemek için kullanılabilir.
Güncelleme: Sabit çekme isteği #83.
Orta önem
[M01] Dinamik bağımsız değişkenlerden sonra iletilen statik bağımsız değişkenler
içinde OrderMixin
sözleşme getTakerAmount
ve getMakerAmount
bayt alanları argüman olarak kullanılır. _callGetTakerAmount
ve _callGetMakerAmount
işlevler. Bu çağrılar, swapın bir tarafını diğer tarafa göre hesaplamanın bir yolunu sunar ve kullanıcıların emirleri kısmen doldurmasına olanak tanır.
The getTakerAmount
/getMakerAmount
alanlar dinamik değişkenlerdir ve alanın önünde paketlenmiştir. takerAmount
ve makerAmount
değerleri _callGetTakerAmount
ve _callGetMakerAmount
işlevler. Kötü niyetli bir oluşturucunun beklenenden daha fazla veri sağlaması mümkündür. getTakerAmount
vegetMakerAmount
itmek için alanlar takerAmount
ve makerAmount
bir sonraki işlevde kodu çözülürken oldukları varsayılan bayt sayısı. Bu, yapıcının, aktarılan alıcı veya yapıcı miktarını tam bayt sağa kaydırmasına ve hatta fazladan 32 baytlık bir veri sağlanırsa bunları tamamen değiştirmesine olanak tanır.
Kullanıcıların zaten manuel olarak incelemesi gerekiyor getTakerAmount
ve getMakerAmount
alanlar sırayla, ancak bu tekniğin fark edilmesi oldukça zordur. Ayrıca şunu belirtmekte fayda var ki, bu saldırı dahili olarak güvenilenler için bile geçerlidir. getMakerAmount
ve getTakerAmount
işlevler. Çoğu saldırı için makul bir eşik tutarının sağlanması fon kaybını önleyecektir.
Bunu önlemek için, dinamik bağımsız değişkenlere statik bağımsız değişkenleri kontrol edecek bir yöntem vermekten kaçınmak amacıyla statik bağımsız değişkenleri dinamik bağımsız değişkenlerden önce kodlamayı düşünün.
Güncelleme: Sabit değil. 1inch ekibi şunları söyledi:
Alıcıların doğrulanmasına ekstra özen göstereceğiz. Potansiyel olarak kötü amaçlı siparişlerin filtrelenmesine yardımcı olacak SDK'mızdaki alıcıların akıl sağlığı doğrulamasını uygulamaya çalışacağız.
[M02] ERC721 emirleri değiştirilebilir
ERC20'lerden daha fazlasını değiştirmek mümkündür. OrderMixin
IERC20'lerle aynı işlev seçiciyi paylaşan bir sözleşme dağıtarak transferFrom
ve bu sözleşmeyi makerAsset
ya da takerAsset
bir sırayla.
Kapsam dışı proxy'ler, yani, ERC721Proxy
, ERC721ProxySafe
, ve ERC1155Proxy
sözleşmeler destek sağlamak için bu modeli takip eder ERC721
ve ERC1155
jetonlar. Proxy'lerin IERC20 ile aynı düzende çağrılması gerektiğinden transferFrom
çağrı, imzanın şununla başlaması gerekir: address from
, address to
ve uint256 amount
. Proxy'lerin gerektirdiği diğer her şey daha sonra iletilebilir ve şu sırayla tanımlanır: makerAssetData
ve takerAssetData
.
ERC1155'ler doğal olarak aynı kimlik belirteçlerinin çoğunu aynı anda aktarabilir; ERC1155Proxy
sözleşmeden faydalanılır amount
alan. Diğer taraftan, ERC721
için bariz bir kullanımı yok amount
alan. Değiştirilemez belirteçleri temsil ettiklerinden, belirli bir tokenId yalnızca bir taneye sahip olacaktır; amount
alan işe yaramaz. Bu nedenle her ikisinin de uygulanması ERC721Proxy
ve ERC721ProxySafe
sözleşmeler gerekli olanı kullanır amount
olarak alan tokenId
yerine.
Bu aşırı yükleme amount
parametre kısmen doldurma olasılığını yaratır ERC721
Ayrı olarak listelenen jetonları indirimli fiyatlarla satın almak için sipariş verin. Örneğin, tek bir kullanıcının birden fazla kullanıcıya sahip olduğu bir durum olabilir. ERC721
tarafından devredilmesine izin verilen aynı sözleşmenin ERC721Proxy
sözleşme yapar ve bunları ayrı limit emirlerinde listeler.
Limit emirlerinin de sağlaması durumunda getMakerAmount
ve getTakerAmount
alanları kısmen doldurmak mümkün olacak ERC721
emirler. Sipariş verildiğinden beri amount
alan aslında şuna karşılık gelir: tokenId
kötü niyetli bir kullanıcı, alana kısmi bir dolgu yerleştirebilir. ERC721
daha yüksek tokenId ile, sonuçta makingAmount
/takingAmount
Bir of ERC721
bu daha düşük bir değere karşılık gelebilir tokenId
. Sonuç ERC721
düşük ile tokenId
fiyatına devredilecektir. (higher tokenId price) * (lower tokenId's id) / (higher tokenId's id)
.
Bu istismarın birkaç gereksinimi vardır:
- çoklu
ERC721
aynı sözleşmeden olanlara izin verilmesiERC721
tek bir sahibin vekili. - Bunlardan biri için açık sipariş
ERC721
bu en düşük değiltokenId
izin verilenlerden. - Siparişte kısmi dolumlara izin verilir.
Kısmi olasılığını tamamen ortadan kaldırmak için ERC721
doldurursanız, ayırmayı düşünün amount
ve tokenId
argümanlar. Tartışmalar ayrı olsun ya da olmasın, kullanıcıları bu davranış konusunda uyarmak ve gelecekte bu modelden kaçınmak için bunu da belgelemeyi düşünün.
Güncelleme: Sabit çekme isteği #59.
[M03] Belgelenmemiş ondalık varsayımlar
The LimitOrderProtocol
sözleşme devralır ChainlinkCalculator
aracılığıyla sözleşme OrderMixin
sözleşme. Bu sözleşme, Chainlink oracle'larının kullanımını mümkün kılmak için iki işlevi ortaya koymaktadır. yüklem kontrolü ve aranması yapıcı miktarı/alıcı miktarı.
Ancak sözleşme, Chainlink oracle'larının raporlaması gereken ondalık sayıların yanı sıra işlev parametrelerinin içermesi gereken ondalık sayıların sayısı hakkında belgelenmemiş varsayımlarda bulunuyor. Bazı senaryolarda bu, varlıkların yanlış fiyatlandırılması ve fonların kasıtsız kaybı gibi beklenmedik davranışlara yol açabilir.
Daha spesifik olarak, sözleşme boyunca örtülü varsayım, Chainlink kahinlerinin 18 ondalık hassasiyetle rapor vereceği yönündedir. Ancak değil tüm Chainlink kahinleri bu ondalık sayıyla rapor edin. Aslında, eğer oracle bir para birimi (örneğin USD) cinsinden bir token çiftini bildirirse, yalnızca 8 ondalık hassasiyete sahip olacaktır. Herhangi bir kısıtlama olmadığından hangi Oracle'lar kullanılabildiğinden, rapor edecekleri ondalık sayıların sayısı hakkında örtülü varsayımlarda bulunulmamalıdır.
Buna bağlı olarak, örtülü bir varsayım vardır: amount
parametresi ChainlinkCalculator
işlevler, yanıltıcı açık bildirimle birlikte 18 ondalık sayı kullanacak singlePrice
işlev Calculates price of token relative to ETH scaled by 1e18
. Gerçekte, bir kehanet olsa bile yok 18 ondalık basamaklı rapor, dönüş değeri singlePrice
fonksiyon ondalık sayıların sayısına göre ölçeklendirilir amount
parametrenin mutlaka 18 ondalık sayı olması gerekmeyebilir.
Benzer şekilde, doublePrice
İşlev, iki Chainlink oracle'ının aynı sayıda ondalık sayıyla rapor vereceğini ve bunun da işlevin sonucunun beklentilerden sapmasına neden olacağını varsayar.
Parametrelerin ve dönüş değerlerinin esas alınması gereken ondalık sayı sayısına ilişkin varsayımları açıkça belgelemeyi düşünün. Ayrıca, ya bu varsayımları bozan kehanetlere dayalı hesaplamaları sınırlamayı ya da ilgili hesaplamaların gerçek ondalık sayıları dikkate almasını sağlamayı düşünün.
Güncelleme: Sabit çekme isteği #75.
Düşük önem derecesi
[L01] Açıkça bildirilmeyen sabitler
Kod tabanında açıklanamayan anlamlarla kullanılan gerçek değerlerin birkaç örneği vardır. Örneğin:
- içinde
OrderMixin
sözleşme_remaining
eşleme anlamsal olarak aşırı yüklenmiştir (sorunda açıklandığı gibi) Haritalamanın anlamsal aşırı yüklenmesi) kısmen doldurulmuş bir sipariş için kalan varlık miktarını takip etmek için yanısıra bir sipariş tamamen yerine getirildiyse. Özellikle,0
bir siparişle ilişkili hiçbir doldurmanın yapılmadığı anlamına gelir,1
bir emrin artık yerine getirilemeyeceği ve bundan daha büyük herhangi bir şey anlamına gelir1
potansiyel olarak yerine getirilebilecek siparişle ilişkili kalan tutarın olduğu anlamına gelir. - içinde
ChainlinkCalculator
sözleşme, gerçek değer1e18
kullanılansinglePrice
fonksiyonu.
Kodun okunabilirliğini artırmak ve yeniden düzenlemeyi kolaylaştırmak için, her sihirli sayı için bir sabit tanımlamayı ve ona açık ve kendini açıklayan bir ad vermeyi düşünün. Karmaşık değerler için, bunların nasıl hesaplandığını veya neden seçildiğini açıklayan bir satır içi yorum eklemeyi düşünün.
Güncelleme: Sabit çekme isteği #75 ve çekme isteği #76.
[L02] Kötü niyetli taraflar izin verilen emirlerin yerine getirilmesini engelleyebilir
The OrderMixin
sözleşme, yapımcı kullanıcıların göndermesine olanak tanır izin verilen siparişler Böylece bunlar, onaylar için ayrı bir işlem yapmak yerine tek bir işlemde yürütülebilir. Ayrıca sipariş alanlar kendi izinlerini sunmak Aynı amaçla siparişin doldurulması sırasında.
Ancak imalatçının izni bu belgenin içinde yer aldığından sipariş, emir doldurma işlemi hafıza havuzundayken hem yapıcının hem de alıcının izinlerine erişilebilir olacaktır. Bu, herhangi bir kötü niyetli kullanıcının bu izinleri almasına ve doldurma işlemini başlatırken bunları ilgili varlık sözleşmelerinde yürütmesine olanak tanıyacaktır. Çünkü bu izinlerin nonce
Çifte harcama saldırısını önlemek için, ön çalıştırma sırasında kullanılan iznin aynısının kullanılmaya çalışılması sonucunda emrin doldurma işlemi başarısız olacaktır.
Herhangi bir güvenlik riski olmamasına ve üreticinin yeni bir emir oluşturup işleme ön onay vermesine rağmen, bu saldırı kesinlikle izin verilen emirlerin kullanılabilirliğini etkileyebilir. Aslında motive olmuş bir saldırgan bloklayabilir herşey Bu saldırıyla izin verilen emirler. Sipariş doldurma sırasında iznin zaten gönderilip gönderilmediğini veya ödeneğin yeterli olup olmadığını doğrulamayı düşünün. Ayrıca sipariş oluşturma sırasında kullanıcıları bu olası saldırı hakkında bilgilendirmeyi de düşünün.
Güncelleme: Sabit değil. 1inch ekibi şunu belirtiyor:
Daha önce onay kontrollerimiz vardı ancak başarısız onayları geri almak için izin akışını basitleştirmeye karar verdik. Yapımcıları konu hakkında bilgilendirmenin yollarını düşüneceğiz.
[L03] Yinelenen kod
Kod tabanında yinelenen kod örnekleri var. Kodun kopyalanması, geliştirme yaşam döngüsünün ilerleyen aşamalarında sorunlara yol açabilir ve projeyi hataların oluşmasına daha yatkın hale getirebilir. Bu tür hatalar, işlevsellik değişiklikleri aynı olması gereken tüm kod örneklerine kopyalanmadığında yanlışlıkla ortaya çıkabilir. Yinelenen kod örnekleri şunları içerir:
Kodu çoğaltmak yerine, kopyalanan kodu içeren tek bir sözleşmeye veya kitaplığa sahip olmayı ve çoğaltılan işlevsellik gerektiğinde onu kullanmayı düşünün.
Güncelleme: Kısmen sabitlendi çekme isteği #60.
[L04] Hatalı veya yanıltıcı test paketi
Test paketinde testlerin beklenen davranıştan saptığı durumlar vardır. Örneğin:
- The
ChainlinkCalculator
sözleşme miras olarak devralındıOrderMixin
sözleşme. Ancak testler sırasındaAmountCalculator.arbitraryStaticCall
işlevi çağırmak için kullanılırChainlinkCalculator
harici, bağımsız bir sözleşme olarak sözleşme. Sonuç beklenen olsa bile test, sistemin mevcut tasarımı ve beklenen kullanım durumu ile ilgili davranışı aşağıdaki çağrıları yaparak yansıtmalıdır:ChainlinkCalculator
keyfi statik çağrıyı kullanmadan doğrudan çalışır. - Proxy sözleşmeleri kapsam dışında olmasına rağmen, protokolü ERC721 varlıklarıyla test ederken,
ERC721Proxy
sözleşme, içindeki varlıkların takası için kullanılmaz test odası.
Test paketinin kendisi bu denetimin kapsamı dışında olduğundan, lütfen tüm testlerin protokolün özelliklerine göre başarılı bir şekilde çalıştığından emin olmak için test paketini kapsamlı bir şekilde incelemeyi düşünün.
Güncelleme: Sabit çekme isteği #57, çekme isteği #59, ve çekme isteği #61.
[L05] Olaylardaki hatalar ve eksiklikler
Kod tabanı boyunca olaylar genellikle sözleşmelerde hassas değişiklikler yapıldığında yayınlanır. Ancak birçok olayda indekslenmiş parametreler eksiktir ve/veya önemli parametreler eksiktir. Örneğin:
Ayrıca aşağıdaki gibi olayların eksik olduğu hassas eylemler de vardır:
Mevcut olayları daha kapsamlı bir şekilde indekslemeyi ve eksik oldukları yerlere yeni parametreler eklemeyi düşünün. Ayrıca tüm olayları, zincir dışı hizmetlerle sözleşmenin durumunu yeniden oluşturmak için kullanılabilecek kadar eksiksiz bir şekilde yayınlamayı düşünün.
Güncelleme: Sabit değil. Ancak 1 inç ekibi bir ekleme yaptı orderRemaining
parametresine OrderCanceled
olay çekme isteği #62.
1inch ekibi şunu belirtiyor:
Ön uç ihtiyaçlarını karşılamak için yalnızca sınırlı bir veri alt kümesinin gerekli olduğunu bulduk. Kapsamlı analiz durumunda, önerilen tüm alanlara izleme yoluyla ulaşılabilir. İçin
OrderRFQMixin
Piyasa yapıcıların hangi siparişlerin iptal edildiğini takip etmek için kendi karmaşık yöntemlerini geliştirmelerini bekliyoruz.
[L06] Olay emisyonu sırasında depolama değişiklikleri
içinde NonceManager
sözleşme ne zaman yapılır NonceIncreased
olay yayılır, mesajı gönderenin nonce'si de artar.
Birden fazla işlemi aynı anda yürütmek, kod tabanı hakkında mantık yürütmeyi zorlaştırabilir, hatalara daha yatkın hale getirebilir ve işlemlerin gözden kaçırılmasına veya yanlış anlaşılmasına yol açabilir.
Kodun genel amaçlılığını, okunabilirliğini ve netliğini geliştirmek için, olayı yayınlamadan önce nonce değerini artırmayı düşünün.
Güncelleme: Sabit çekme isteği #63.
[L07] Tutarsız kod çözme metodolojileri sonuç farklılıklarına neden olabilir
Genişletilebilirliğini ve esnekliğini desteklemek için Limit Order Protokolü rutin olarak dinamik bayt verileriyle ve harici sözleşmelerden gelen keyfi dönüş değerleriyle uğraşmak zorundadır. Sonuç olarak, protokol şunları içerir: ArgumentsDecoder
dinamik bayt değerlerini temel veri türlerine daha verimli bir şekilde dönüştürmek için kitaplık. Ancak bu kütüphane özel olarak kullanılmaz ve bazı durumlarda abi.decode
onun yerine kullanılır. Ek olarak, bazı sözleşmeler kullanılıyor abi coder v1
diğerleri kullanırken abi coder v2
. İlki, diğerine daha benzer bir performans sergiliyor ArgumentsDecoder
kütüphane, ikincisi ise kod çözme sırasında ek kontroller gerçekleştirir.
Bu farklı kod çözme metodolojilerinin tutarsız kullanımı, kod tabanının amacı ile gerçek davranışı arasında ince farklılıklara neden olabilir.
Örneğin, simulateCalls
işlevi yalnızca şunu kullanır: ArgumentsDecoder.decodeBool
işlevi. Eğer simulateCalls
işlevi, bir sıranın yüklem kısmında yapılacak çağrıları kontrol etmek için kullanılırsa, farklı kod çözme metodolojileri kullanıldığı için sonuçları, yüklem koşulları değerlendirildiğinde gerçekte ortaya çıkanlardan farklı olabilir.
Yani, örneğin, eğer bir yüklem dışsal bir etki yapıyorsa staticcall
döndüren bazı işlevlere uint256
değeri beklenenden birden büyük bool
, o zaman bu çağrı geri dönecektir çünkü dönüş değeri ile kodu çözüldü abi coder v2
'ler abi.decode
gibi değerleri kabul etmeyecek olan bool
. Ancak, tam olarak aynı arama yapılırsa simulateCalls
, sonra basitçe şu şekilde işaretlenecektir: true
Çünkü, decodeBool
sıfırdan büyük herhangi bir değeri şu şekilde ele alır: true
.
Yapmak için simulateCalls
işlev, gerçek yüklem çağrılarının davranışını tam olarak yansıtır; onu kullanacak şekilde değiştirmeyi düşünün abi.decode
.
Güncelleme: Sabit çekme isteği #82.
[L08] Giriş doğrulama eksikliği
The fillOrderToWithPermit
ve fillOrderTo
fonksiyonları OrderMixin
sözleşmenin yanı sıra fillOrderRFQToWithPermit
ve fillOrderRFQTo
fonksiyonları OrderRFQMixin
sözleşmeyi doğrulamayın target
adres parametresi.
Bu, bir kullanıcının istemeden sıfır adresi geçmesine ve bunun sonucunda siparişi doldurduktan sonra alması gereken varlıkları kilitlemesine olanak tanır.
Kullanıcıların yanlışlıkla paralarını kilitlemediklerinden emin olmak için, target
adres belirtilen işlevlerdeki sıfır adrese eşit değildir.
Güncelleme: Sabit çekme isteği #78.
[L09] Düşük birim test kapsamı
Tüm proje için birim test kapsamı %75 civarında olup, bazı sözleşmelerin kapsamı özellikle düşüktür.
Kodu doğrulamak ve yeni özellikleri yeniden düzenlerken ve geliştirirken regresyonları önlemek için birim testlerinin önemini göz önünde bulundurarak, birim testi kapsamının önemli ölçüde en az %95'e çıkarılmasını ve olası olmayan durumları bile kapsayan uç durumların dahil edilmesini teşvik ediyoruz.
Güncelleme: Sabit değil.
[L10] Yanıltıcı veya eksik satır içi belgeler
Kod tabanı boyunca birkaç yanıltıcı ve/veya eksik satır içi belgeleme örneği tespit edildi ve bunların düzeltilmesi gerekiyor.
Aşağıdakiler yanıltıcı satır içi dokümantasyon örnekleridir:
- içinde
ChainlinkCalculator
sözleşmesinglePrice
işlevi NatSpec@notice
etiket öyle olduğunu söylüyorCalculates price of token relative to ETH scaled by 1e18
, ama aslında sonuç şu: değer ofamount
ölçeklendirilmiş jetonlar1e18
Oracle'ın ETH cinsinden rapor vermeyebileceği durumlarda (örneğin, ETH içermeyen bir çift için). - içinde
OrderRFQMixin
sözleşmeinvalidatorForOrderRFQ
işlevi NatSpec@return
etiket yanıltıcıdır, çünkü ilgili geçersiz kılıcı bitin ayarlanması için teklif doldurulmamış olabilir. Sipariş iptal edilmiş de olabilir. - Hatlarda 147, 165, ve 188 of
OrderMixin.sol
, NatSpec@param
Etiketler dilbilgisine aykırıdır. - İnternet üzerinden 20 of
ERC1155Proxy.sol
,@notice
etiketi, hesaplanan karmanın karma işleminin sonucu olduğunu belirtir.func_733NCGU
işlevi olması gerektiği yerdefunc_301JL5R
yerine işlev.
Aşağıdakiler eksik satır içi dokümantasyon örnekleridir:
- İçindeki işlevler
AmountCalculator
Sözleşme hiçbir parametreyi açıklamamaktadır. - içinde
ChainlinkCalculator
sözleşmesinglePrice
vedoublePrice
işlevler tüm parametreleri açıklamaz. - içinde
ImmutableOwner
sözleşme, genel değişken ve değiştiricinin NatSpec'i yoktur. - içinde
InteractiveNotificationReceiver
sözleşmenotifyFillOrder
fonksiyon hiçbir parametreyi açıklamaz. - içinde
LimitOrderProtocol
sözleşmeDOMAIN_SEPARATOR
işlevin NatSpec'i yok. - Olaylar ve haritalamalar
NonceManager
NatSpec yok. - içinde
OrderRFQMixin
sözleşmecancelOrderRFQ*
işlevler dönüş değerlerini açıklamaz. - içinde
OrderMixin
sözleşme, bazı işlevler tam NatSpec'ten yoksundur. - İnternet üzerinden 168 of
OrderMixin.sol
ve çevrimiçi 71 ofOrderRFQMixin.sol
, eksik@dev
etiketi. - İçindeki işlevler
PredicateHelper
sözleşme tüm parametreleri açıklamamaktadır.
Açık satır içi belgeler, kodun amacını özetlemek için temeldir. Satır içi dokümantasyon ile uygulama arasındaki uyumsuzluklar, sistemin nasıl davranması beklendiği konusunda ciddi yanılgılara yol açabilir. Geliştiricilerin, kullanıcıların ve denetçilerin kafa karışıklığını önlemek için bu hataları düzeltmeyi düşünün.
Güncelleme: Kısmen sabitlendi. Yanıltıcı belgeler ele alındı çekme isteği #75 ve çekme isteği #77.
1inch ekibi şunu belirtiyor:
Yanıltıcı dokümanları düzelttik. Dokümanların tamamlanması daha sonra yapılacaktır.
[L11] Kanca kullanıldığında DoS siparişleri mümkün
The OrderMixin
Sözleşme, başarıları için koşullara sahip olabilecek genel zincir dışı takas emirlerini karşılama işlevini uygular. Sipariş doldurma sırasında sipariş, önceden tanımlanmış "yüklem" koşullarını kontrol edin infaz işlemine devam etmeden önce.
Bununla birlikte, bu yüklem koşulları herhangi bir keyfi sözleşmenin mantığını hedef alabileceğinden, kötü niyetli bir yapıcı, alıcıları bir emrin doğru davrandığına ve zincir dışı kontrol edilirken geçerli olduğuna inandırmak için kandırabilir, ancak daha sonra aynı emri doldurmaya çalışırken başarısız olabilir. zincir üzerinde. Yüklem davranışındaki bu değişiklik, yüklemlerin bağlı olduğu bazı değişken durumların önden çalıştırılmasıyla, gönderilen gazın veya hatta hangi adreslerin çağrıya dahil olduğunun incelenmesiyle veya başka bir mantıkla yapılabilir.
Ayrıca, eğer yapımcı bir tanımlamışsa takas sırasında etkileşim, interactionTarget
sözleşme, başarılı bir sipariş doldurmayı engellemek için kendini geri döndürebilir veya ödeneği iptal edebilir, bu da esasen kötü niyetli yüklemlerle aynı sonuca yol açar.
Varlıklar risk altında olmasa da, uygun bir emir bulan kullanıcılar veya botlar, görünürde meşru görünebilecek bu tür spam emirlerini tanımlamaya çalışmanın yükünü artıracaktır. Bu tür siparişleri tespit edememeleri durumunda boşa giden gaz maliyetlerine maruz kalacaklar. Spam siparişlerinin miktarını azaltmak için bu kancalara yönelik mevcut hedefleri kısıtlamayı düşünün. Ayrıca, siparişleri doldurmaya çalışmadan önce kullanıcıları bu olasılık konusunda uyarmayı da göz önünde bulundurun.
Güncelleme: Sabit değil. 1inch ekibi şunu belirtiyor:
Bunu arka uçta ele alacağız ve olası alıcıları sorun hakkında bilgilendirmenin yollarını düşüneceğiz.
[L12] Yuvarlama aşağıdakiler için elverişsiz olabilir: taker
içinde OrderMixin
ve OrderRFQMixin
Bir emir yerine getirilirken ve alıcı sadece bir emir verdiğinde sözleşmeler makingAmount
or takingAmount
miktar, protokol takasın karşı tutarını hesaplamaya çalışır.
Bu hesaplamalarla ilgili iki sorun vardır; bunlardan ilki, miktar parametrelerinin kullanması gereken ondalık sayıların sayısını sınırlayan hiçbir belge veya mantığın bulunmamasıdır. Belgelenmemiş ondalık varsayımlar konu.
İkinci husus ise bu hesaplamalar sırasında protokol turlarının yapımcı lehine sonuçlanmasıdır. Örtülü ondalık varsayımlar bozulduğunda yuvarlama sorunu büyük ölçüde daha da kötüleşebilir, ancak her şey beklenen şartlarda olsa bile yuvarlama küçük, tek sayılarla gerçekleşir.
Alıcının minimum tutarı belirtmesine izin vermeyi düşünün makerAsset
azami tutarla birlikte almaya razı oldukları varlık takerAsset
Herhangi bir yuvarlamanın kabulü daha açık olsun diye takas etmeye istekli oldukları varlık.
Güncelleme: Sabit değil. 1inch ekibi şunu belirtiyor:
Eşik miktarı alıcının korunması için yeterli olmalıdır.
[L13] Parametreler eksik olduğunda çelişkili sipariş yönetimi
içinde OrderMixin
sözleşme fillOrderTo
işlev dahili çağrılar yapar _callGetMakerAmount
ve _callGetTakerAmount
bir doldurma girişiminde bulunulduğunda çalışır ve makingAmount
ya da takingAmount
parametreler sırasıyla sıfırsa veya makingAmount
değeri bundan daha büyük remainingMakerAmount
değeri.
The _callGetMakerAmount
ve _callGetTakerAmount
sipariş şu şekilde oluşturulmadıysa çağrılar geri dönüşlere yol açacaktır: getMakerAmount
or getTakerAmount
sırasıyla parametreler ve kısmi doldurma yürütülüyor.
An yanında satır içi yorum _callGetMakerAmount
ve bir yanında satır içi yorum _callGetTakerAmount
siparişin şu şekilde oluşturulmaması durumunda "yalnızca tam doldurmalara izin verildiğini" iddia edin getMakerAmount
or getTakerAmount
parametreleri.
Ancak bunun geçerli olmadığı kod yolları da vardır çünkü bu yollar kodu kontrol etmez. length
her ikisinden de getMakerAmount
ve getTakerAmount
parametreleri.
spesifik olarak, zaman taker
belirtir takerAmount
yalnızca bir siparişin değeri getMakerAmount
bu çağrı olmadığı sürece getMakerAmount
şundan daha büyük bir miktar döndürür: remainingMakerAmount
satır içi belgelere aykırı olarak kısmi bir doldurma gerçekleştirilebilir.
Bu, bu kod yollarının amacını belirsiz bırakır. Beklenen davranış buysa, satır içi belgeleri daha açık olacak şekilde değiştirmeyi düşünün. Bu kasıtsız bir davranışsa her zaman her iki bağlantının da uzunluğunu kontrol etmeyi düşünün. getMakerAmount
ve getTakerAmount
parametreleri eş zamanlı olarak değiştirerek uygulamanın satır içi belgelerde tanımlanan davranışı güçlendirmesini sağlayın.
Güncelleme: Sabit çekme isteği #79.
[L14] Kullanımdan kaldırılan Chainlink çağrılarını kullanma
The ChainlinkCalculator
Sözleşmenin Chainlink kahinlerini sorgulamak için kullanılması amaçlanıyor. Bunu onlara çağrı yaparak yapar. latestTimestamp
ve latestAnswer
yöntemleri, ikisi de kullanımdan kaldırıldı. Aslında yöntemler artık Chainlink toplayıcılarının API'sinde mevcut değil üçüncü versiyon itibariyle.
Chainlink oracles ile gelecekteki olası uyumsuzlukları önlemek için, latestRoundData
bunun yerine yöntem.
Güncelleme: Sabit çekme isteği #67.
Notlar ve Ek Bilgiler
[N01] Arayüzler içe aktarılmıyor
The AggregatorInterface
arayüz, kopyalanan kodun bir alt kümesi gibi görünüyor ChainLink
'nin genel kod deposu. Tam arayüz dahildir ChainLink
'nin sözleşmesi npm paketi.
Mümkün olduğunda, başka bir projenin arayüzlerini yeniden tanımlamak ve/veya yeniden yazmak yerine arayüz uyumsuzlukları ve bunun sonucunda ortaya çıkan sorunlar olasılığını azaltmak için, bunun yerine resmi npm paketleri aracılığıyla yüklenen arayüzleri kullanmayı düşünün.
Güncelleme: Sabit çekme isteği #66.
[N02] Kullanımdan kaldırılan proje bağımlılıkları
Kurulum sırasında projenin bağımlılıkları, NPM kurulu paketlerden birinin uyardığını, Highlight
, "artık desteklenmeyecek veya gelecekte güvenlik güncellemeleri alınmayacak".
Bu paketin bir güvenlik riskine neden olması pek olası olmasa da, bu paketi kullanan bağımlılığı bakımı yapılan bir sürüme yükseltmeyi düşünün.
Güncelleme: Sabit çekme isteği #64.
[N03] Yöntemleri görüntülemek için yapılan harici çağrılar statik çağrılar değildir
Kod tabanının çoğunda protokol açıkça OpenZeppelin aracılığıyla harici aramalar yapar. functionStaticCall
beklenmediğinde veya istenmediğinde durum değişiklikleri olasılığını kısıtlama yöntemi. Ancak, ChainlinkCalculator
sözleşme, yalnızca harici aramalar yapma niyetine rağmen view
Chainlink oracle'larındaki yöntemler, harici çağrılar singlePrice
ve doublePrice
işlevler açık bir şekilde yapılmaz staticcall
s.
Bundan kaynaklanan acil bir güvenlik endişesi tespit etmemiş olsak da, saldırı yüzeyini azaltmak, tutarlılığı geliştirmek ve amacı netleştirmek için açık bir şekilde kullanmayı düşünün. staticcall
s, tüm harici aramalar için view
içindeki fonksiyonlar ChainlinkCalculator
sözleşme.
Güncelleme: Sabit değil. 1inch ekibi şunu belirtiyor:
Sözdizimi karmaşıklığının tutarlılıktaki iyileştirmeleri geçersiz kıldığını düşünüyoruz.
[N04] Geçersiz siparişlerle erken başarısızlığa uğramamak
içinde OrderMixin
sözleşme fillOrderTo
işlev, bir siparişin daha önce gönderilmemesi durumundaki özel durumu ele alır (remainingMakerAmount == 0
), ancak siparişin artık geçerli olmadığı durumu açıkça ele almaz (remainingMakerAmount == 1
).
İkinci senaryoda, işlev eninde sonunda eski durumuna dönecektir, ancak bu yalnızca önemsiz miktarda gaz yakıldıktan sonra gerçekleşir. Amacı netleştirmek, okunabilirliği artırmak ve gaz kullanımını azaltmak için geçersiz sıra senaryosunu işlevin başlangıcına doğru açıkça ele almayı düşünün.
Güncelleme: Sabit çekme isteği #68.
[N05] Özet olarak işaretlenmeyen yardımcı sözleşmeler
Solidity'de anahtar kelime abstract
Kendi başlarına işlevsel sözleşme olmayan veya bu şekilde kullanılması amaçlanmayan sözleşmeler için kullanılır. Yerine, abstract
sözleşmeler, bağımsız işlevsel sözleşmeler oluşturmak için sistemdeki diğer sözleşmeler tarafından devralınır.
Kod tabanı boyunca, kendi başlarına dağıtılmaları amaçlanmasa da soyut olarak işaretlenmeyen yardımcı sözleşme örnekleri vardır. Örneğin, AmountCalculator
, ChainlinkCalculator
, ImmutableOwner
, NonceManager
, ve PredicateHelper
Sözleşmelerin tümü, devralınan sözleşmeler tarafından kullanılması amaçlanan temel işlevler kümesinden oluşur.
Yardımcı sözleşmeleri şu şekilde işaretlemeyi düşünün: abstract
yalnızca onları devralan sözleşmelere işlevsellik eklemek amacıyla tasarlandıklarını açıkça belirtmek.
Güncelleme: Sabit değil. 1inch ekibi şunu belirtiyor:
Bu yardımcılar ayrı ayrı konuşlandırılabilir. Yalnızca gaz tasarrufu için miras alınırlar.
[N06] Tutarsız fonksiyon sıralaması
Kod tabanı boyunca işlev sıralaması genellikle şu şekildedir: Solidity Stil Kılavuzu'nda önerilen sıralama, hangisi: constructor
, fallback
, external
, public
, internal
, private
.
Ancak, OrderMixin
sözleşme public
checkPredicate
işlev, stili ikiye bölerek stil kılavuzundan sapar. external
fonksiyonlar.
Projenin genel okunabilirliğini geliştirmek için, Solidity Style Guide tarafından önerildiği gibi kod tabanı boyunca işlev sıralamasını standartlaştırmayı düşünün.
Güncelleme: Sabit çekme isteği #69.
[N07] Tutarsız sipariş doldurma akışı
The OrderMixin
ve RFQOrderMixin
Sözleşmelerin her ikisi de imzalı emirlerin doldurulmasını yönetir, ancak iki sözleşme arasındaki genel emir akışı tutarsızdır.
OrderMixin
'ler fillOrderTo
işlev bu genel akışı takip eder (sözde kod):
if ((takingAmount == 0) == (makingAmount == 0))
else if (takingAmount == 0)
else (handle makingAmount == 0) THEN swapTokens
Oysa RFQOrderMixin
benzer fillOrderRFQTo
işlev bu akışı takip eder (sözde kod):
if (takingAmount == 0 && makingAmount == 0)
else if (takingAmount == 0)
else if (makingAmount == 0)
else revert THEN swapTokens
Bu iki fonksiyonun her birindeki ilk koşulun neden farklı olduğuna veya neden farklı olduğuna dair belgelerde hiçbir bilgi yok. takingAmount
ve makingAmount
ikinci fonksiyonda her ikisi de sıfır olamaz. Ayrıca her ikisinin de olduğu durum makingAmount
ve takingAmount
hakkında mantık yürütmek çok daha kolaydır fillOrderRFQTo
işlevi, finalde açıkça ele alındığı için else
engellemek.
Amacı netleştirmek ve kodun genel okunabilirliğini artırmak için, ya bu iki sözleşmedeki genel sipariş akışını standartlaştırmayı ya da farklılıkların neden bulunduğunu açıkça belgelemeyi düşünün.
Güncelleme: Sabit değil. 1inch ekibi şunu belirtiyor:
Bunun nedeni limitli emirlerdeki özel fiyatlandırma fonksiyonlarıdır. O zamandan beri
getMakerAmount
potansiyel olarak önemli ölçüde farklılık gösterebilirgetTakerAmount
, alıcıların farklı olacağı durumlarda muhtemelen kafalarını karıştıracağından, alıcı için varsayılan seçeneği yapmamanın daha iyi olacağını düşündük.
[N08] Hata mesajları tutarsız biçimde biçimlendirilmiş veya yanıltıcı
Kod tabanı boyunca, require
ve revert
Bir işlemin başarısız olmasına neden olan belirli koşullar hakkında kullanıcıları bilgilendirmeyi amaçlayan hata mesajlarının tutarsız bir şekilde biçimlendirildiği tespit edildi.
Örneğin satırlardaki hata mesajlarının her biri 85 bölgesinin OrderMixin.sol
, 16 bölgesinin ERC721ProxySafe.sol
, ve 26 bölgesinin Permitable.sol
farklı bir üslup kullanın.
Ayrıca bazı hata mesajları yanıltıcıdır:
Hata mesajları, kullanıcıları arıza durumları hakkında bilgilendirmeyi amaçlamaktadır; bu nedenle, sistemle etkileşimde bulunmak için uygun düzeltmelerin yapılabilmesi için yeterli bilgi sağlamalıdırlar. Bilgilendirici olmayan hata mesajları genel kullanıcı deneyimine büyük ölçüde zarar vererek sistemin kalitesini düşürür. Üstelik tutarsız biçimde biçimlendirilmiş hata mesajları gereksiz kafa karışıklığına neden olabilir. Bu nedenle, her şeyin doğru olduğundan emin olmak için kod tabanının tamamını gözden geçirmeyi düşünün. require
ve revert
bildiriminde tutarlı biçimde biçimlendirilmiş, doğru, bilgilendirici ve kullanıcı dostu bir hata mesajı bulunur.
Güncelleme: Kısmen sabitlendi çekme isteği #81.
[N09] Adlandırılmış dönüş değişkenlerinin tutarsız kullanımı
Adlandırılmış dönüş değişkenlerinin tutarsız bir kullanımı var OrderMixin
sözleşme. Bazı işlevler adlandırılmış değişkenleri döndür, diğerleri açık değerleri döndür, ve diğerleri adlandırılmış bir dönüş değişkeni bildirin ancak onu geçersiz kılın açık bir return ifadesi ile.
Adlandırılmış tüm dönüş değişkenlerini kaldırarak, bunları açıkça yerel değişkenler olarak bildirerek ve uygun olan yerlere gerekli dönüş ifadelerini ekleyerek kod tabanı boyunca değerleri döndürmek için tutarlı bir yaklaşım benimsemeyi düşünün. Bu, kodun hem açıklığını hem de okunabilirliğini artıracak ve aynı zamanda gelecekteki kod yeniden düzenlemeleri sırasındaki regresyonların azaltılmasına da yardımcı olabilecektir.
Güncelleme: Sabit çekme isteği #73.
[N10] Siparişin hash hesaplaması API'ye açık değil
The external
fonksiyonlar remaining
, remainingRaw
ve remainingsRaw
hepsi başarılı operasyon için bir sipariş karması bekliyor.
Ancak yardımcı fonksiyon _hash
Bir siparişin karmasını döndüren , private
görünürlük. Bu, kullanıcıların bir siparişin karmasını elde etmek için siparişlerin bazı kısımlarını ve etki alanı dizelerini manuel olarak paketlemeleri gerektiği anlamına gelir.
Sipariş karmalarını hesaplarken olası hatalardan kaçınmak ve kullanıcılara bir emrin ilgili karma değerini oluşturmaya yönelik bir yöntem sağlamak için, sipariş karmalarının görünürlüğünü genişletmeyi düşünün. _hash
işlev public
ve adı yeniden düzenleme hash
kodun geri kalanıyla tutarlı olması için.
Güncelleme: Sabit çekme isteği #74.
[N11] Eşlemenin anlamsal aşırı yüklenmesi
The _remaining
haritalama OrderMixin
sözleşme, siparişlerin durumunu ve bu siparişler için kullanılabilir kalan varlık miktarını izlemek için anlamsal olarak aşırı yüklenmiştir.
Üstlenebileceği üç durum şunlardır:
0
: Sipariş karması henüz görülmedi.1
: Sipariş ya iptal edildi ya da tamamen yerine getirildi.- herhangi
uint
daha geniş1
: KalanmakerAmount
sipariş üzerine doldurulabilir artı1
.
Bu semantik aşırı yükleme, bu değerin paketlenmesi ve açılmasını gerektirir. lookup
, cancellation
, initialization
, ve storage
.
Semantik aşırı yükleme ve bunu etkinleştirmek için gereken mantık, hataya açık olabilir ve kod tabanının anlaşılmasını ve üzerinde mantık yürütmeyi zorlaştırabilir; aynı zamanda kodun gelecekteki güncellemelerinde gerilemelerin kapısını da açabilir.
Kodun okunabilirliğini artırmak için siparişlerin tamamlanma durumunu ayrı bir eşlemede izlemeyi düşünün.
Güncelleme: Sabit değil. 1inch ekibi, bir düzeltmenin gaz maliyetlerini artıracağını belirtti fillOrder
fonksiyonu.
[N12] İzinli emirler keyfi sözleşmelere çağrı yapılmasına izin verir
The OrderMixin
sözleşme devralır Permitable
kabul eden varlıklarla tek işlemli emir doldurmaya izin veren sözleşme permit
ödeneklerin değiştirilmesi çağrısında bulundu.
Bununla birlikte, çağrılar Permitable
sözleşme Hedefin izin verilebilir bir varlık olup olmadığını veya kötü niyetli bir kullanıcının, sipariş doldurma tamamlanmadan önce başka bir çağrı gerçekleştirebilecek keyfi bir sözleşmenin adresini iletmesine izin verebilecek bir varlık olup olmadığını doğrulamaz.
Sözleşme olmasına rağmen yeniden girişe karşı korumalıSaldırı yüzeyinin azaltılması ve yürütme sırasında harici sözleşmelerin çağrılmasının engellenmesi her zaman tavsiye edilir. İzne dahil olan varlığı, siparişe dahil olan varlıklarla veya protokol için bir varlık beyaz listesiyle sınırlamayı düşünün.
Güncelleme: Sabit değil. 1inch ekibi şunu belirtiyor:
OrderMixin
aslında gerçek tokenlar hakkında bilgi yokmakerAsset
vetakerAsset
bazen vekiller veya diğer ara sözleşmeler olabilir ve gerçek tokenlar hakkındaki bilgiler bazı keyfi baytlarda saklanır. Dolayısıyla hangi varlığı kısıtlamanın geçerli bir yolu yokpermit
çağrılır.
[N13] solhint
hiçbir zaman yeniden etkinleştirilmez
Kod tabanı boyunca birkaç tane var solhint-disable
beyanlar, özellikle çevrimiçi olanlar 23 ve çevrimiçi 41 of RevertReasonParser.sol
ile sonlandırılmayan solhint-enable
.
Açıklıktan yana olmak ve devre dışı bırakırken mümkün olduğunca kısıtlayıcı olmak solhint
, kullanmayı düşünün solhint-disable-line
or solhint-disable-next-line
bunun yerine çizgiye benzer 16 aynı dosyanın.
Güncelleme: Sabit çekme isteği #72.
[N14] Yazım hataları
Kod tabanı aşağıdaki yazım hatalarını içerir:
Ayrıca projenin README
(bu denetimin kapsamı dışında) aşağıdaki yazım hatalarını içeriyor:
Kodun okunabilirliğini artırmak için bu yazım hatalarını düzeltmeyi düşünün.
Güncelleme: Sabit çekme isteği #71 ve çekme isteği #77.
[N15] Kullanımı uint
yerine uint256
Açıklığı desteklemek için, tüm örnekler uint
olarak beyan edilmelidir uint256
. Özellikle, içinde olanlar for
çizgilerdeki döngüler 98 ve 119 of OrderMixin.sol
ve çizgiler 16 ve 30 of PredicateHelper.sol
.
Güncelleme: Sabit çekme isteği #70.
Sonuç
Yüksek önem derecesine sahip 3 sorun bulundu. En iyi uygulamaları takip etmek ve potansiyel saldırı yüzeyini azaltmak için bazı değişiklikler önerildi.
- &
- 7
- Hakkımızda
- erişim
- Göre
- Hesap
- karşısında
- Hareket
- eylemler
- Ek
- adres
- avantaj
- Türkiye
- Izin
- zaten
- Rağmen
- tutarları
- analiz
- api
- yaklaşım
- argümanlar
- etrafında
- varlık
- Varlıklar
- denetim
- Arka uç
- Başlangıç
- olmak
- İYİ
- en iyi uygulamalar
- Bit
- botlar
- inşa etmek
- çağrı
- hangi
- durumlarda
- Sebeb olmak
- Zincir bağlantı
- değişiklik
- denetleme
- Çekler
- kod
- karmaşık
- koşul
- karışıklık
- kas kütlesi inşasında ve
- içeren
- sözleşme
- sözleşmeleri
- Düzeltmeler
- maliyetler
- olabilir
- Çift
- yaratıcı
- Para birimi
- akım
- veri
- anlaşma
- Hizmet Reddi
- dağıtma
- Dizayn
- geliştiriciler
- gelişme
- DID
- farklılık
- farklı
- domain
- çift
- dinamik
- Erken
- kenar
- teşvik etmek
- özellikle
- ETH
- Etkinlikler
- olaylar
- her şey
- örnek
- takas
- beklenen
- deneyim
- sömürmek
- Özellikler
- Alanlar
- Nihayet
- Ad
- sabit
- Esneklik
- akış
- takip et
- bulundu
- tam
- işlev
- para
- gelecek
- oyun
- GAZ
- genel
- Verilmesi
- harika
- rehberlik
- kullanma
- esrar
- karma
- sahip olan
- yardım et
- Yüksek
- büyük ölçüde
- Ne kadar
- HTTPS
- melez
- belirlemek
- darbe
- uygulamak
- önemli
- ithal
- dahil
- Dahil olmak üzere
- Artırmak
- artmış
- bilgi
- bilgi
- Altyapı
- anlayışlar
- niyet
- faiz
- arayüzey
- ilgili
- sorunlar
- IT
- büyük
- büyük
- öncülük etmek
- önemli
- Kütüphane
- Sınırlı
- çizgi
- Likidite
- Listelenmiş
- Listeler
- yerel
- baktı
- arama
- büyük
- yapıcı
- Yapımı
- pazar
- mempool
- ayna
- model
- çoğu
- En popüler
- yani
- Yeni Özellikler
- Aynen
- mantarsız belirteçler
- resmi
- açık
- Operasyon
- seçenek
- kehanet
- sipariş
- emir
- Diğer
- sahip
- model
- Popüler
- mevcut
- önlenmesi
- fiyat
- fiyatlandırma
- özel
- süreç
- proje
- Projeler
- koruma
- protokol
- sağlamak
- sağlar
- vekil
- halka açık
- yayınlamak
- satın alma
- kalite
- yükseltmek
- Gerçeklik
- azaltmak
- güven
- rapor
- Raporlar
- Depo
- Yer Alan Kurallar
- DİNLENME
- Sonuçlar
- İade
- yorum
- Risk
- mermi
- koşmak
- sdk
- güvenlik
- Hizmetler
- set
- Paylaşılan
- Paylar
- çalışma
- benzer
- Basit
- küçük
- akıllı
- Akıllı Sözleşmeler
- So
- katılık
- Spam
- özellikle
- Harcama
- Spot
- başlama
- Eyalet
- Açıklama
- Devletler
- Durum
- hafızası
- stil
- gönderilen
- başarı
- başarılı
- Başarılı olarak
- destek
- destekli
- yüzey
- anahtar
- sistem
- Hedef
- test
- Test yapmak
- testleri
- İçinden
- boyunca
- zaman
- birlikte
- simge
- Jeton
- iz
- Takip
- işlem
- Güven
- benzersiz
- Güncellemeler
- us
- KULLANILABİLİRLİK
- USD
- USDT
- kullanıcılar
- yarar
- değer
- Görüntüle
- görünürlük
- beklemek
- Ne
- Beyaz Liste
- içinde
- olmadan
- İş
- değer
- sıfır