16 كانون الأول، 2021
المُقدّمة
• 1inch طلب منا فريق مراجعة وتدقيق بروتوكول طلب الحد v2 العقود الذكية. نظرنا إلى الكود ونشرنا نتائجنا الآن.
مجال
دققنا الالتزام 4d94eea25e4dac6271bfd703096a5c4a4d899b4a
ل 1inch/limit-order-protocol
مخزن. في النطاق كانت العقود التالية:
- 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
تم أيضًا استبعاد جميع ملفات وأدلة المشروع الأخرى (بما في ذلك الاختبارات) ، جنبًا إلى جنب مع التبعيات والمشاريع الخارجية ، ونظرية اللعبة ، وتصميم الحوافز ، من نطاق هذا التدقيق. كان من المفترض أن تعمل تبعيات الكود الخارجي والعقد على أنها موثقة ، ويفترض أن الخدمات الخلفية المقدمة من قبل 1 بوصة تعمل في مصلحة البروتوكول.
الصحة العامة
بشكل عام ، وجدنا أن قاعدة بيانات المشروع قابلة للقراءة ومنظمة بشكل جيد ، على الرغم من أنها يمكن أن تستفيد من وثائق أكثر شمولاً ، خاصة حول كتل كود التجميع ، وحالات حافة البروتوكول ، والأصول / المسندات / الموارد الخارجية التي سيتم استخدامها ، مسؤوليات / قيود الخدمة الخلفية المقدمة ، والتفاعلات بين الجهات الفاعلة. يبذل المشروع قصارى جهده لجعل الإجراءات فعالة في استخدام الغاز ، وأحيانًا مع المخاطرة بجعل التفكير في الكود أكثر صعوبة ؛ نثير القضايا المتعلقة بذلك أدناه. طوال عملية التدقيق ، كان الفريق الذي يبلغ حجمه 1 بوصة متاحًا للغاية وسريع الاستجابة ومن السهل جدًا التعامل معه.
نبذة عن النظام
يمكّن بروتوكول الحد من الطلب makers
لتوقيع الطلبات خارج السلسلة لمقايضات الرموز. يسهل البروتوكول بعد ذلك ملء الطلبات الموقعة مسبقًا حسب الأمر takers
. الطلبات قابلة للتوسيع بدرجة كبيرة ، ويمكنها استدعاء العقود الخارجية بشكل ثابت في عدة نقاط خلال عملية ملء الطلب. هذه القابلية للتوسعة تضفي على البروتوكول فائدة ، لكنها تضيف تعقيدًا وسطح هجوم أكبر للأوامر نفسها.
من المهم ملاحظة أنه لا يوجد تخزين على السلسلة لتفاصيل الطلب. لا يتم تتبع حالة ملء الطلبات أو إلغائها إلا من خلال تجزئات الطلب. هذا يستلزم أن تكون الطلبات مشتركة بين نظير إلى نظير أو عبر حزب مركزي. في هذه الحالة ، يعتزم فريق 1 بوصة العمل كطرف مركزي ، حيث يقوم بتجميع الطلبات الموقعة واستخدام تلك الطلبات كمصدر للسيولة لبروتوكولاتهم الأخرى. سيتم نشر الطلبات عبر واجهة برمجة التطبيقات الخاصة بهم حتى يتمكن المستخدمون من التفاعل معها.
تمنح هذه المركزية الفريق الذي يبلغ حجمه 1 بوصة تحكمًا شديدًا في الأوامر التي يتم نشرها وتنفيذها في النهاية. يمنحهم هذا أيضًا القدرة على فرض رقابة على الطلبات ، والتي قد تكون مفيدة في حالة الأوامر الخبيثة أو الخادعة ، ولكن يمكن أيضًا إساءة استخدامها والسماح لهم بتوجيه أي مستخدم آخر في حالة وجود أمر مؤات من خلال عدم إظهاره من خلال واجهة برمجة التطبيقات.
أدوار مميزة
على الرغم من أن العقود التي يتم استخدام الدور فيها كانت خارج النطاق ، تم تحديد دور مميز واحد. ان immutableOwner
تم تعيينه لمنشئ عقد الوكيل في وقت الإنشاء ، ويتم استخدامه لتقييد الوصول إلى الوكيل external
الوظائف.
التبعيات الخارجية وافتراضات الثقة
يتطلب تصميم هذا البروتوكول مكونات خارج السلسلة وعلى السلسلة ، ويمكن استخدام هذا النموذج الهجين للتخفيف من بعض نواقل الهجوم التي نحددها في تقريرنا ، ولكن تكلفة هذه القدرة هي زيادة الاعتماد على الفريق والبنية التحتية بحجم 1 بوصة.
بالإضافة إلى ذلك ، يوفر بروتوكول Limit Order Protocol وظائف تهدف إلى استرداد الأسعار من Chainlink oracles. لقد افترضنا أن هذه الأوراكل صادقة ، ويمكن الوصول إليها ، وتعمل بشكل صحيح.
علاوة على ذلك ، نظرًا لمرونة الطلب ، هناك العديد من نقاط الاتصال بالعقود الخارجية التي لم يتم التحقق من صحتها. هذا يعني أنه يمكن لمستخدم ضار إساءة استخدام هذه المكالمات وانتحال صفة المسندات أو الأصول أو الأوراكل باستخدام عقود ضارة من أجل تنفيذ الإجراءات أثناء تنفيذ الأوامر. على الرغم من أن المشروع محمي في بعض المناطق ضد إعادة الدخول ، يمكن أن تتسبب هذه النواقل في رفض الخدمة أو أوامر البريد العشوائي غير المكتشفة. يدرك فريق 1 بوصة أن بعض المشكلات قد تظهر عند استخدام عقود غير مألوفة للبروتوكول وقد أشاروا إلى نيتهم أن يتم دعم الأصول "الممتازة" الرئيسية فقط بواسطة المشروع. ومع ذلك ، تجدر الإشارة إلى أنه حتى مع الأصول الأكثر شيوعًا ، هناك سلوكيات جوهرية من كل أصل قد تسبب مشاكل في البروتوكولات التي لا تعالجها بشكل صحيح ، مثل فرض رسوم أثناء التحويلات باستخدام USDT أو إرجاع رمز خطأ بدلاً من منطقية للنجاح مع cTokens.
النتائج
هنا نقدم النتائج التي توصلنا إليها.
شدة حرجة
لا شيء.
خطورة شديدة
[H01] تم تمرير بيانات غير متسقة إلى _makeCall
في مجلة OrderMixin
العقد _makeCall
تستخدم وظيفة لنقل الأصول من الآخذ إلى الصانع وثم من صانع إلى آخذ. في النقل الأخير ، فإن _makeCall
تم تمرير الوظيفة بشكل غير صحيح في الأمر makerAsset
كمعامل أخير ، عندما يجب أن يكون الأمر makerAssetData
.
نتيجة لذلك ، فإن أي وظيفة وكيل تعتمد على makerAssetData
سوف تنكسر الحجة.
لتكون متسقة مع المكالمة السابقة لـ _makeCall
ولدعم وظائف الوكيل بشكل كامل ، ضع في اعتبارك تحديث order.makerAsset
المعلمة ل order.makerAssetData
.
تحديث: الثابتة في طلب سحب # 57.
[H02] يمكن لأي شخص تنفيذ الطلبات الخاصة المملوءة جزئيًا
يسمح البروتوكول بإنشاء أوامر خاصة وعامة. في الطلبات الخاصة ، فقط ملف allowedSender
العنوان ، الذي حدده الصانع أثناء إنشاء الأمر ، قادر على ملء الطلب.
ومع ذلك ، في OrderMixin
عقد، التحقق من صحة allowedSender
العنوان تم تحديد النطاق بشكل غير صحيح ، مما يعني أنه يتم تقييمه فقط داخل المنطق الذي يتعامل مع التعبئة الأولى للأمر. إذا تم ملء طلب خاص جزئيًا ، فسيتم فحص allowedSender
لم يعد من الممكن الوصول إلى العنوان وأصبح الطلب قابلاً للتنفيذ من قبل أي شخص.
لتوضيح النية حول ما إذا كان يجب على أي مستخدم أن يكون قادرًا على ملء الطلبات الخاصة المملوءة جزئيًا أم لا ، ضع في اعتبارك إما توثيق سبب السلوك الحالي أو التحقق من صحة allowedSender
عنوان خارج نطاق التعبئة الأولى لضمان التحقق من صحته في كل مرة تتم فيها محاولة التعبئة.
تحديث: الثابتة في طلب سحب # 58.
[H03] يمكن للصانع الخبيث الاستفادة من عمليات التعبئة الجزئية لسرقة أصول المستفيد
أوامر من OrderMixin
يمكن ملء العقد جزئيًا. لدعم عمليات التعبئة الجزئية ، يتطلب البروتوكول طريقة لحساب جانبي المقايضات. كلاهما getMakerAmount
و getTakerAmount
يتم تحديد الحقول من قبل صانع الطلب لهذا الغرض بالضبط.
عند ملء أحد الطلبات ، يجب على المتقدمين تقديم إما makingAmount
أو ال takingAmount
القيم وكذلك أ thresholdAmount
القيمة. هناك مساران مختلفان للتشفير يمكن اتباعهما ، بناءً على ما إذا كان makingAmount
أو ال takingAmount
تم توفيرها.
أول واحد عندما makingAmount
يتم تعريف المعلمة. ممكن بتر ال makingAmount
قيمة وكذلك احسب takingAmount
قيمة لذلك. في هذه الحالة ، فإن thresholdAmount
يضمن أن takingAmount
القيمة المأخوذة ليست كبيرة بشكل غير متوقع.
والثاني عندما takingAmount
يتم تعريف المعلمة. في مثل هذه الحالة ، سوف احسب makingAmount
القيمة ، مع إمكانية اقتطاعها و إعادة حساب takingAmount
قيمة إذا حدث ذلك. في هذه الحالة ، فإن thresholdAmount
القيمة تضمن أن makingAmount
القيمة التي تم إرجاعها هي ليست صغيرة بشكل غير متوقع.
توجد طريقتان للاستغلال ، تنفرد كل منهما بأحد مسارات الكود المذكورة سابقًا. تتطلب أساليب الاستغلال هذه الخبيثة getMakerAmount
و getTakerAmount
المهام. تنفيذ بسيط لهذه الوظائف سيكون لها نفس السلوك AmountCalculator
الصورة getMakerAmount
و getTakerAmount
وظائف ، ولكن باستخدام مفتاح ذي تشفير ثابت سيجبرهم على إرجاع قيمة يتحكم فيها المهاجم عند الحاجة.
يتضمن نمط الاستغلال الأول ، الأقل خطورة ، مسار الكود الأول حيث makingAmount
القيمة المحددة في ترتيب التعبئة. سينتظر صانع ضار أمر ملء يحدد makingAmount
لتظهر في mempool من أجل مقدمتها. سوف يستنزفون كل القيمة باستثناء 1 من جانب الصانع ثم يجبرون _callGetTakerAmount
لإرجاع المبلغ المحدد في المستخدم thresholdAmount
القيمة (أو مخصصاتها إذا كانت أقل). عندما تتم معاملة المستخدم أخيرًا ، فسيتم تبديله بالكامل thresholdAmount
قيمة takerAsset
لوحدة واحدة من makerAsset
. هذا الاستغلال محدود بالمبلغ الذي قدمه thresholdAmount
قيمة أو مقدار takerAsset
سمح المستخدم على LimitOrderProtocol
العقد.
يتضمن نمط الاستغلال الثاني الأكثر خطورة المسار الكود الثاني حيث takingAmount
القيمة المحددة. وبالمثل ، سينتظر المُصنِّع الضار أمر ملء يحدد ملف takingAmount
قيمة لتظهر في mempool. سيقومون بتشغيل الصفقة وإجبار makingAmount
القيمة التي تم إرجاعها بواسطة _callGetMakerAmount
تعمل لتكون أعلى من كل من remainingMakerAmount
و thresholdAmount
. سوف يقومون أيضًا بتعيين ملف takingAmount
تم إرجاع القيمة بواسطة _callGetTakerAmount
ليكون مقدار takerAsset
الأصل المسموح به على LimitOrderProtocol
من قبل الآخذ. عندما تتم معاملة المستفيد ، فسوف يتم ذلك اقتطاع ال makingAmount
قيمة وبعد ذلك إعادة حساب takingAmount
القيمة. لا يُضمن أن تكون إعادة الحساب هذه أقل ، وفي هذه الحالة سوف تستنزف المتلقي كل takerAsset
التي سمحوا بها في العقد. في مسار الكود هذا ، يكون ملف thresholdAmount
القيمة التأكد من أن makingAmount
ليس منخفضًا جدًا، لذلك أخذ كل الآخذ takerAsset
الأصل غير محدد. يتم تقييد الأموال المفقودة بمبلغ takerAsset
الأصل الذي سمح به المستخدم على LimitOrderProtocol
العقد.
هذه الثغرات ليست ممكنة بدون أوامر جزئية ، وبشكل أكثر تحديدًا ، أوامر جزئية مع ضارة getMakerAmount
و getTakerAmount
التنفيذ.
القضية الرئيسية ل thresholdAmount
التحقق من القيمة هو أنه يغطي جانبًا واحدًا فقط من المقايضة ، ولكن يمكن التلاعب بالجانب الآخر عبر التشغيل الأمامي. لا توجد تأكيدات على أن القيمة التي اقترحها المتلقي في الأصل تظل دون تغيير. ضع في اعتبارك إزالة makingAmount
الاقتطاع من كل من مسارات التعليمات البرمجية والعودة إذا كان الطلب لا يدعم تعبئة بالحجم المطلوب. من خلال القيام بذلك ، فإن thresholdAmount
يمكن استخدامها لتقييد الجانب الآخر من المقايضة بشكل كافٍ وتجنب السلوك غير المتوقع ، حتى في الأوامر الخبيثة.
تحديث: الثابتة في طلب سحب # 83.
شدة متوسطة
[M01] مرت الحجج الثابتة بعد الحجج الديناميكية
في مجلة OrderMixin
العقد getTakerAmount
و getMakerAmount
يتم استخدام حقول البايت كوسيطات لـ _callGetTakerAmount
و _callGetMakerAmount
المهام. توفر هذه الاستدعاءات طريقة لحساب جانب واحد من المقايضة بناءً على الجانب الآخر ، وتسمح للمستخدمين بتعبئة الطلبات جزئيًا.
• getTakerAmount
/getMakerAmount
الحقول متغيرات ديناميكية ومعبأة أمام takerAmount
و makerAmount
القيم في _callGetTakerAmount
و _callGetMakerAmount
المهام. من الممكن أن يقوم صانع ضار بتوفير بيانات أكثر مما هو متوقع في ملف getTakerAmount
و getMakerAmount
الحقول لدفع takerAmount
و makerAmount
البايتات الماضية حيث يفترض أن تكون عند فك تشفيرها في الوظيفة التالية. يسمح هذا للمُصنِّع بتحويل كمية المتلقي أو المُصنِّع التي تم تمريرها بمقدار بايت كامل إلى اليمين وحتى استبدالها تمامًا إذا تم توفير 32 بايتًا إضافيًا من البيانات.
يتعين على المستخدمين بالفعل مراجعة ملف getTakerAmount
و getMakerAmount
الحقول بالترتيب ، ولكن من الصعب تحديد هذه التقنية. وتجدر الإشارة أيضًا إلى أن هذا الهجوم ينطبق حتى على الأشخاص الموثوق بهم داخليًا getMakerAmount
و getTakerAmount
المهام. بالنسبة لمعظم الهجمات ، فإن توفير مبلغ حد معقول سيمنع فقدان الأموال.
لمنع ذلك ، ضع في اعتبارك ترميز الحجج الثابتة قبل الوسيطات الديناميكية لتجنب إعطاء الوسيطات الديناميكية طريقة للتحكم في الحجج الثابتة.
تحديث: لم تحل. ذكر فريق 1 بوصة:
سنأخذ عناية إضافية عند التحقق من صحة الحروف. سنحاول تنفيذ التحقق من صحة البيانات في sdk الخاص بنا والذي سيساعد في تصفية الطلبات التي يحتمل أن تكون ضارة.
[M02] يمكن التلاعب بأوامر ERC721
من الممكن تبادل أكثر من مجرد ERC20s عبر OrderMixin
من خلال نشر عقد يشترك في نفس محدد الوظيفة مثل IERC20 transferFrom
، وتقديم هذا العقد باسم makerAsset
أو ال takerAsset
بالترتيب.
الوكلاء خارج النطاق ، وهي ERC721Proxy
, ERC721ProxySafe
و ERC1155Proxy
تتبع العقود هذا النمط لتقديم الدعم لـ ERC721
و ERC1155
الرموز. نظرًا لأنه يجب استدعاء الوكلاء بنفس النمط مثل IERC20 transferFrom
الاتصال ، يجب أن يبدأ التوقيع بـ address from
, address to
و uint256 amount
. يمكن تمرير أي شيء آخر تتطلبه الوكلاء بعد ذلك ، ويتم تعريفه بالترتيب على أنه makerAssetData
و takerAssetData
.
يمكن لـ ERC1155s نقل العديد من الرموز المميزة للمعرف نفسه في وقت واحد بشكل طبيعي ، مما يعني أن ملف ERC1155Proxy
العقد يستخدم amount
ميدان. على الجانب الآخر، ERC721
ليس لديك استخدام واضح لـ amount
ميدان. نظرًا لأنها تمثل رموزًا غير قابلة للاستبدال ، فلن يكون هناك سوى رمز مميز واحد فقط ، مما يجعل amount
مجال عديم الفائدة. وبسبب هذا ، فإن التنفيذ على حد سواء ERC721Proxy
و ERC721ProxySafe
استخدام العقود المطلوبة amount
المجال مثل tokenId
بدلا من ذلك.
هذا التحميل الزائد amount
المعلمة تخلق إمكانية الملء الجزئي ERC721
أوامر لشراء الرموز المميزة المدرجة بشكل منفصل بأسعار مخفضة. على سبيل المثال ، يمكن أن تكون هناك حالة يكون فيها لمستخدم واحد عدة مرات ERC721
s من نفس العقد المسموح بنقلها من قبل ERC721Proxy
العقد وسردها في أوامر حد منفصلة.
إذا كانت أوامر الحد توفر أيضًا getMakerAmount
و getTakerAmount
الحقول ، سيكون من الممكن ملء هذه الحقول جزئيًا ERC721
الطلب #٪ s. منذ الأمر amount
الحقل يتوافق في الواقع مع tokenId
، يمكن للمستخدم الضار وضع تعبئة جزئية لملف ERC721
ذات الرمز المميز الأعلى ، مما يؤدي إلى أ makingAmount
/takingAmount
ل ERC721
يمكن أن يتوافق مع أقل tokenId
. والنتيجة هي ERC721
مع الجزء السفلي tokenId
بسعر (higher tokenId price) * (lower tokenId's id) / (higher tokenId's id)
.
هذا الاستغلال له بعض المتطلبات:
- قد يؤدي إجراء
ERC721
ق من نفس العقد مسموح به على أي منهماERC721
بالوكالة من قبل مالك واحد. - أمر مفتوح لأحد
ERC721
هذا ليس هو الأدنىtokenId
المسموح بها. - يسمح بالملء الجزئي للأمر.
لإزالة إمكانية جزئية تماما ERC721
يملأ ، ضع في اعتبارك فصل amount
و tokenId
الحجج. سواء كانت الحجج منفصلة أم لا ، ضع في اعتبارك أيضًا توثيق ذلك لتنبيه المستخدمين بهذا السلوك ولتجنب هذا النمط في المستقبل.
تحديث: الثابتة في طلب سحب # 59.
[M03] افتراضات عشرية غير موثقة
• LimitOrderProtocol
العقد يرث ChainlinkCalculator
التعاقد من خلال OrderMixin
عقد. يعرض هذا العقد وظيفتين لتمكين استخدام Chainlink أوراكل أثناء تحقق المسند والبحث عن كمية صانع/كمية آخذ.
ومع ذلك ، فإن العقد يضع افتراضات غير موثقة حول عدد الكسور العشرية التي يجب أن يبلغ عنها Chainlink oracles ، بالإضافة إلى عدد الكسور العشرية التي يجب أن تحتويها معلمات الوظيفة. في بعض السيناريوهات ، قد يؤدي ذلك إلى سلوكيات غير متوقعة ، بما في ذلك سوء تسعير الأصول والخسارة غير المقصودة للأموال.
وبشكل أكثر تحديدًا ، في جميع أنحاء العقد ، فإن الافتراض الضمني هو أن سلسلة أوراكل تشينلينك ستبلغ بدقة 18 كسرًا عشريًا. ومع ذلك ، لا كل تشينلينك أوراكل تقرير بهذا العدد من الكسور العشرية. في الواقع ، إذا أبلغت أوراكل عن زوج من التوكنات من حيث العملة (الدولار الأمريكي ، على سبيل المثال) ، فسيكون لها فقط 8 أرقام عشرية من الدقة. نظرًا لعدم وجود قيود على التي يمكن استخدام أوراكل ، ولا ينبغي وضع افتراضات ضمنية حول عدد الكسور العشرية التي سيبلغون بها.
وبالمثل ، هناك افتراض ضمني بأن amount
معلمة لـ ChainlinkCalculator
ستستخدم الدوال 18 رقمًا عشريًا ، جنبًا إلى جنب مع الإعلان الصريح المضلل بأن singlePrice
وظيفة Calculates price of token relative to ETH scaled by 1e18
. في الواقع ، حتى مع ذلك أوراكل هل تقرير بـ 18 رقمًا عشريًا ، القيمة المرجعة لـ singlePrice
سيتم تحجيم الدالة من خلال عدد الكسور العشرية في amount
المعلمة ، والتي قد لا تكون بالضرورة 18 رقمًا عشريًا.
وبالمثل، فإن doublePrice
تفترض الدالة أن اثنين من رموز Chainlink أوراكل سيبلغان بنفس العدد من الكسور العشرية ، مما يتسبب في انحراف نتيجة الدالة عن التوقعات.
ضع في اعتبارك التوثيق الصريح للافتراضات المتعلقة بعدد الكسور العشرية التي يجب أن تكون المعلمات وقيم الإرجاع من حيث. علاوة على ذلك ، ضع في اعتبارك إما تقييد الحسابات التي تعتمد على أوراكل التي تكسر تلك الافتراضات ، أو أن تأخذ الحسابات ذات الصلة العدد الفعلي من الكسور العشرية في الاعتبار.
تحديث: الثابتة في طلب سحب # 75.
شدة منخفضة
[L01] لم يتم التصريح عن الثوابت صراحة
هناك عدد قليل من تكرارات القيم الحرفية المستخدمة ذات المعنى غير المبرر في قاعدة الرموز. علي سبيل المثال:
- في مجلة
OrderMixin
العقد_remaining
تم تحميل رسم الخرائط بشكل زائد (كما هو موضح في المشكلة التحميل الزائد الدلالي لرسم الخرائط) لتتبع مقدار الأصل المتبقي لأمر معبأ جزئيًا وأيضا إذا تم تنفيذ الطلب بالكامل. على وجه التحديد،0
يعني أنه لم يتم إجراء أي عمليات تنفيذ مرتبطة بأمر ما ،1
يعني أنه لم يعد من الممكن تنفيذ أمر ما ، وأي شيء أكبر من1
يعني أن هناك مبلغًا متبقيًا مرتبطًا بالطلب يمكن ملؤه. - في مجلة
ChainlinkCalculator
العقد ، القيمة الحرفية1e18
يستخدم فيsinglePrice
وظيفة.
لتحسين قراءة الكود وتسهيل إعادة البناء ، ضع في اعتبارك تحديد ثابت لكل رقم سحري ، ومنحه اسمًا واضحًا وواضحًا. للقيم المعقدة ، ضع في اعتبارك إضافة تعليق مضمّن يشرح كيفية حسابها أو سبب اختيارها.
تحديث: الثابتة في طلب سحب # 75 و طلب سحب # 76.
[L02] يمكن للأطراف الخبيثة منع تنفيذ الأوامر المسموح بها
• OrderMixin
يسمح العقد للمستخدمين الصانعين بالإرسال الأوامر المسموح بها بحيث يمكن تنفيذها في معاملة واحدة ، بدلاً من الاضطرار إلى الحصول على معاملة منفصلة للموافقات. أيضا ، يمكن لأخذ الطلب تقديم تصريحهم الخاص أثناء ملء الطلب لنفس الغرض.
ومع ذلك ، لأن تصريح الصانع وارد داخل طلب، يمكن الوصول إلى تصاريح كل من المُصنِّع والمُتخذ أثناء وجود معاملة تعبئة الطلب في مجموعة الذاكرة. هذا من شأنه أن يجعل من الممكن لأي مستخدم ضار أن يأخذ تلك التصاريح وينفذها في عقود الأصول ذات الصلة أثناء تشغيل معاملة التعبئة. لأن هذه التصاريح لها nonce
لمنع هجوم الإنفاق المزدوج ، ستفشل معاملة تنفيذ الطلب نتيجة لمحاولة استخدام نفس التصريح الذي تم استخدامه للتو أثناء العرض الأمامي.
على الرغم من عدم وجود مخاطر أمنية ، ويمكن للمصنع إنشاء طلب جديد والموافقة المسبقة على المعاملة ، إلا أن هذا الهجوم قد يؤثر بالتأكيد على قابلية استخدام الأوامر المسموح بها. في الواقع ، يمكن للمهاجم المتحمس أن يصد من جميع الأوامر المسموح بها بهذا الهجوم. ضع في اعتبارك التحقق مما إذا كان التصريح قد تم تقديمه بالفعل ، أو إذا كان البدل كافياً ، أثناء تنفيذ الطلب. ضع في اعتبارك أيضًا السماح للمستخدمين بمعرفة هذا الهجوم المحتمل أثناء تكوين الطلب.
تحديث: لم تحل. يقول الفريق 1 بوصة:
لقد أجرينا فحوصات الموافقة من قبل ولكننا قررنا تبسيط تدفق التصاريح لمجرد الرجوع إلى الموافقات غير الناجحة. سنفكر في طرق إخطار صانعي هذه المشكلة.
[L03] رمز مكرر
هناك حالات من التعليمات البرمجية المكررة داخل مصدر البرنامج. يمكن أن يؤدي تكرار الكود إلى حدوث مشكلات لاحقًا في دورة حياة التطوير ويترك المشروع أكثر عرضة لإدخال الأخطاء. يمكن تقديم مثل هذه الأخطاء عن غير قصد عندما لا يتم تكرار تغييرات الوظائف عبر جميع مثيلات التعليمات البرمجية التي يجب أن تكون متطابقة. تتضمن أمثلة التعليمات البرمجية المكررة ما يلي:
بدلاً من تكرار الكود ، ضع في اعتبارك وجود عقد أو مكتبة واحدة فقط تحتوي على الكود المكرر واستخدامه كلما كانت الوظيفة المكررة مطلوبة.
تحديث: ثابت جزئيًا في طلب سحب # 60.
[L04] مجموعة اختبار خاطئة أو مضللة
هناك حالات في مجموعة الاختبار حيث تنحرف الاختبارات عن سلوكها المتوقع. على سبيل المثال:
- •
ChainlinkCalculator
العقد موروثة منOrderMixin
عقد. ومع ذلك ، خلال الاختباراتAmountCalculator.arbitraryStaticCall
تستخدم الوظيفة لاستدعاءChainlinkCalculator
العقد كعقد خارجي مستقل. على الرغم من أن النتيجة هي النتيجة المتوقعة ، يجب أن يعكس الاختبار السلوك مع التصميم الحالي للنظام وحالة الاستخدام المتوقعة عن طريق الاتصالChainlinkCalculator
يعمل مباشرة دون استخدام المكالمة الثابتة التعسفية. - على الرغم من أن عقود الوكيل كانت خارج النطاق ، فقد لاحظنا أنه عند اختبار البروتوكول باستخدام أصول ERC721 ، فإن
ERC721Proxy
لا يتم استخدام العقد لمبادلة الأصول في حزمة اختبار.
نظرًا لأن مجموعة الاختبار نفسها خارج نطاق هذا التدقيق ، يرجى النظر في مراجعة مجموعة الاختبار بدقة للتأكد من تشغيل جميع الاختبارات بنجاح وفقًا لمواصفات البروتوكول.
تحديث: الثابتة في طلب سحب # 57, طلب سحب # 59و طلب سحب # 61.
[L05] أخطاء وسهو في الأحداث
في جميع أنحاء قاعدة البيانات ، يتم إصدار الأحداث بشكل عام عند إجراء تغييرات حساسة على العقود. ومع ذلك ، تفتقر العديد من الأحداث إلى معلمات مفهرسة و / أو تفتقد إلى معلمات مهمة. علي سبيل المثال:
هناك أيضًا إجراءات حساسة تفتقر إلى الأحداث ، مثل:
ضع في اعتبارك إجراء فهرسة كاملة للأحداث الموجودة وإضافة معلمات جديدة في الأماكن التي تفتقر إليها. أيضًا ، ضع في اعتبارك إرسال جميع الأحداث بطريقة كاملة بحيث يمكن استخدامها لإعادة بناء حالة العقد من خلال الخدمات خارج السلسلة.
تحديث: لم تحل. ومع ذلك ، قام فريق 1inch بإضافة ملف orderRemaining
المعلمة إلى OrderCanceled
حدث في طلب سحب # 62.
يقول الفريق 1 بوصة:
وجدنا أن مجموعة فرعية محدودة فقط من البيانات مطلوبة لتلبية احتياجات الواجهة الأمامية. في حالة التحليل الشامل ، تتوفر جميع الحقول المقترحة عبر التتبع. بالنسبة
OrderRFQMixin
نتوقع من صانعي السوق بناء طريقتهم المتطورة لتتبع الطلبات التي تم إلغاؤها.
[L06] تغييرات التخزين أثناء انبعاث الحدث
في مجلة NonceManager
العقد ، عندما NonceIncreased
ينبعث الحدث ، يتم أيضًا زيادة قيمة nonce لمرسل الرسالة.
يمكن أن يؤدي تنفيذ عمليات متعددة في وقت واحد إلى جعل قاعدة الشفرة أكثر صعوبة في التفكير ، وأكثر عرضة للأخطاء ، ويمكن أن يؤدي إلى التغاضي عن العمليات أو إساءة فهمها.
لتحسين القصد العام ، وسهولة القراءة ، ووضوح الكود ، ضع في اعتبارك زيادة قيمة nonce قبل إصدار الحدث.
تحديث: الثابتة في طلب سحب # 63.
[L07] قد تتسبب منهجيات فك التشفير غير المتسقة في حدوث تناقضات في النتائج
لدعم كل من قابليته للتوسعة والمرونة ، يتعين على "بروتوكول الحد الأقصى" بشكل روتيني التعامل مع بيانات البايت الديناميكية وقيم الإرجاع التعسفية من العقود الخارجية. نتيجة لذلك ، يتضمن البروتوكول ملف ArgumentsDecoder
مكتبة لتحويل قيم البايت الديناميكية بشكل أكثر كفاءة إلى أنواع بيانات أساسية. ومع ذلك ، لا يتم استخدام هذه المكتبة حصريًا ، وفي بعض الحالات abi.decode
يستخدم بدلا من ذلك. بالإضافة إلى ذلك ، تستخدم بعض العقود abi coder v1
بينما يستخدمه الآخرون abi coder v2
. السابق يؤدي بشكل أكثر تشابهًا مع ArgumentsDecoder
مكتبة ، بينما يقوم الأخير بإجراء فحوصات إضافية عند فك التشفير.
يمكن أن يؤدي الاستخدام غير المتسق لمنهجيات فك التشفير المختلفة هذه إلى تناقضات دقيقة بين النية والسلوك الفعلي لقاعدة الكود.
على سبيل المثال، simulateCalls
تستخدم الوظيفة فقط ArgumentsDecoder.decodeBool
وظيفة. إذا كان simulateCalls
تُستخدم الوظيفة للتحقق من المكالمات التي سيتم إجراؤها في الجزء الأصلي من الأمر ، ومن ثم يمكن أن تنحرف نتائجها عما يحدث بالفعل عند تقييم الشروط الأصلية ، نظرًا لاستخدام منهجيات مختلفة لفك التشفير.
لذلك ، على سبيل المثال ، إذا كان المسند يصنع عنصرًا خارجيًا staticcall
لبعض الوظائف التي ترجع أ uint256
قيمة أكبر من واحد بدلاً من المتوقع bool
، ثم ستعود تلك المكالمة ، لأن القيمة المعادة هي فك مع abi coder v2
الصورة abi.decode
التي لن تقبل مثل هذه القيم مثل bool
. ومع ذلك ، إذا تم إجراء نفس المكالمة بالضبط مع simulateCalls
، ثم أنه سيتم ببساطة وضع علامة true
، لان decodeBool
يعامل أي قيمة أكبر من الصفر على أنها true
.
لتصنع simulateCalls
تعكس الوظيفة بشكل كامل سلوك المكالمات الأصلية الفعلية ، فكر في تعديلها لاستخدامها abi.decode
.
تحديث: الثابتة في طلب سحب # 82.
[L08] عدم التحقق من صحة الإدخال
• fillOrderToWithPermit
و fillOrderTo
وظائف OrderMixin
العقد ، وكذلك fillOrderRFQToWithPermit
و fillOrderRFQTo
وظائف OrderRFQMixin
العقد ، لا تتحقق من صحة target
معلمة العنوان.
هذا يجعل من الممكن للمستخدم تمرير العنوان صفر عن غير قصد ، ونتيجة لذلك ، يحبس الأصول التي من المفترض أن يتلقاها بعد ملء الطلب.
لضمان عدم قيام المستخدمين بحبس أموالهم عن طريق الخطأ ، ضع في اعتبارك التحقق من صحة ملف target
العنوان لا يساوي العنوان الصفري في الوظائف المذكورة.
تحديث: الثابتة في طلب سحب # 78.
[L09] تغطية اختبار وحدة منخفضة
تبلغ تغطية اختبار الوحدة للمشروع بأكمله حوالي 75 ٪ ، مع وجود تغطية منخفضة لبعض العقود بشكل خاص.
بالنظر إلى أهمية اختبارات الوحدة للتحقق من صحة الكود ومنع الانحدار عند إعادة بناء وتطوير ميزات جديدة ، نشجع زيادة تغطية اختبار الوحدة بشكل كبير إلى 95٪ على الأقل ، بما في ذلك حالات الحافة التي تغطي حتى المواقف غير المحتملة.
تحديث: لم تحل.
[L10] وثائق مضمنة مضللة أو غير كاملة
في جميع أنحاء قاعدة البيانات ، تم تحديد عدد قليل من حالات التوثيق المضمّن المضلل و / أو غير المكتمل ويجب إصلاحها.
فيما يلي أمثلة على التوثيق المضمّن المضلل:
- في مجلة
ChainlinkCalculator
العقدsinglePrice
المهام ناتسبيك@notice
بطاقة يقول ذلكCalculates price of token relative to ETH scaled by 1e18
، ولكن في الواقع ، نتيجته هي قيمنا ofamount
الرموز التي تم تحجيمها بواسطة1e18
، حيث قد لا يُبلغ oracle من حيث ETH (بالنسبة للزوج الذي لا يتضمن ETH ، على سبيل المثال). - في مجلة
OrderRFQMixin
العقدinvalidatorForOrderRFQ
المهام ناتسبيك@return
بطاقة مضلل ، لأن الاقتباس ربما لم يتم ملؤه لتعيين بت المفسد المعني. يمكن أيضًا إلغاء الطلب. - على الخطوط 147, 165و 188 of
OrderMixin.sol
، NatSpec@param
العلامات غير نحوية. - عبر الانترنت 20 of
ERC1155Proxy.sol
أطلقت حملة@notice
تنص العلامة على أن التجزئة المحسوبة هي نتيجة تجزئة ملفfunc_733NCGU
وظيفة ، حيث يجب أن يكونfunc_301JL5R
تعمل بدلا من ذلك.
فيما يلي أمثلة على الوثائق المضمنة غير المكتملة:
- وظائف في
AmountCalculator
العقد لا يصف أي من المعلمات. - في مجلة
ChainlinkCalculator
العقدsinglePrice
وdoublePrice
لا تصف الدالات جميع المعلمات. - في مجلة
ImmutableOwner
العقد ، المتغير العام والمعدل ليس لهما NatSpec. - في مجلة
InteractiveNotificationReceiver
العقدnotifyFillOrder
لا تصف الوظيفة أيًا من المعلمات. - في مجلة
LimitOrderProtocol
العقدDOMAIN_SEPARATOR
الوظيفة لا تحتوي على NatSpec. - الأحداث والتعيينات في
NonceManager
ليس لديهم NatSpec. - في مجلة
OrderRFQMixin
عقد،cancelOrderRFQ*
الدوال لا تصف قيم الإرجاع. - في مجلة
OrderMixin
العقد ، العديد من الوظائف تفتقر إلى NatSpec الكامل. - عبر الانترنت 168 of
OrderMixin.sol
وعلى الإنترنت 71 ofOrderRFQMixin.sol
، فإنه يفتقد@dev
العلامة. - وظائف في
PredicateHelper
العقد لا يصف جميع المعلمات.
يعد التوثيق الواضح المضمّن أمرًا أساسيًا لتحديد مقاصد الكود. يمكن أن يؤدي عدم التطابق بين الوثائق المضمنة والتنفيذ إلى مفاهيم خاطئة خطيرة حول الطريقة التي يُتوقع أن يتصرف بها النظام. ضع في اعتبارك إصلاح هذه الأخطاء لتجنب إرباك المطورين والمستخدمين والمدققين على حدٍ سواء.
تحديث: تم إصلاحه جزئيًا. الوثائق المضللة التي تم تناولها في طلب سحب # 75 و طلب سحب # 77.
يقول الفريق 1 بوصة:
لقد أصلحنا المستندات المضللة. سيتم الانتهاء من المستندات في وقت لاحق.
[L11] أوامر DoS ممكنة عند استخدام الخطافات
• OrderMixin
ينفذ العقد وظائف لملء أوامر المقايضة العامة خارج السلسلة والتي يمكن أن يكون لها شروط لنجاحها. أثناء تنفيذ الأوامر ، يمكن للأمر تحقق من شروط "المسند" المحددة مسبقًا قبل الاستمرار في التنفيذ.
ومع ذلك ، نظرًا لأن هذه الشروط الأصلية يمكن أن تستهدف منطق أي عقد تعسفي ، يمكن للصانع الخبيث أن يخدع المتلقين للاعتقاد بأن الأمر يتصرف بشكل صحيح وأنه صالح عند التحقق منه خارج السلسلة ، ولكنه يفشل بعد ذلك عند محاولة ملء نفس الطلب على السلسلة. يمكن إجراء هذا التغيير في السلوك الأصلي عن طريق التشغيل الأمامي لبعض الحالات المتغيرة التي تعتمد عليها المسندات ، عن طريق فحص الغاز المرسل أو حتى العناوين المتضمنة في المكالمة ، أو من خلال منطق آخر.
علاوة على ذلك ، إذا حدد الصانع أ التفاعل أثناء المبادلةأطلقت حملة interactionTarget
يمكن أن يتراجع العقد عن نفسه أو يلغي البدل لمنع تنفيذ ناجح للأمر ، مما يؤدي بشكل أساسي إلى نفس النتيجة مثل المسندات الخبيثة.
على الرغم من أن الأصول لن تكون في خطر ، فإن المستخدمين أو الروبوتات الذين يجدون أمرًا مناسبًا سيكون لديهم عبء متزايد لمحاولة تحديد هذه الأنواع من أوامر البريد العشوائي التي قد تبدو مشروعة على السطح. في حالة فشلهم في تحديد هذه الأنواع من الطلبات ، فسوف يتحملون تكاليف الغاز المهدر. لتقليل كمية أوامر البريد العشوائي ، ضع في اعتبارك تقييد الأهداف المتاحة لهذه الخطافات. ضع في اعتبارك أيضًا تحذير المستخدمين بشأن هذا الاحتمال قبل محاولتهم ملء الطلبات.
تحديث: لم تحل. يقول الفريق 1 بوصة:
نحن نتعامل مع ذلك في الخلفية الخاصة بنا وسنفكر في طرق إخطار المحتجزين المحتملين بشأن هذه المشكلة.
[L12] يمكن أن يكون التقريب غير مواتٍ لـ taker
في مجلة OrderMixin
و OrderRFQMixin
العقود ، عندما يتم تنفيذ الأمر ويقدم المتلقي فقط a makingAmount
or takingAmount
المبلغ ، يحاول البروتوكول حساب المبلغ المقابل للمبادلة.
هناك مشكلتان في هذه الحسابات ، الأولى هي عدم وجود وثائق أو منطق يحد من عدد الكسور العشرية التي يجب أن تستخدمها معلمات المبلغ ، والتي تناولناها في افتراضات عشرية غير موثقة القضية.
المسألة الثانية هي أنه في سياق هذه الحسابات ، يتم عمل البروتوكول لصالح الصانع. يمكن أن تتفاقم مشكلة التقريب بشكل كبير عندما يتم كسر الافتراضات العشرية الضمنية ، ولكن حتى عندما يكون كل شيء في الشروط المتوقعة ، سيحدث التقريب بكميات صغيرة وفردية.
ضع في اعتبارك السماح للمتلقي بتحديد الحد الأدنى لمبلغ makerAsset
الأصل الذي يرغبون في الحصول عليه مع حد أقصى takerAsset
الأصل الذي يرغبون في مبادلته ، بحيث يكون قبول أي تقريب أكثر وضوحًا.
تحديث: لم تحل. يقول الفريق 1 بوصة:
يجب أن يكون مبلغ الحد كافيًا لحماية المستفيد.
[L13] التعامل مع الأمر المتناقض عند نقص المعلمات
في مجلة OrderMixin
العقد fillOrderTo
وظيفة تجعل المكالمات الداخلية إلى _callGetMakerAmount
و _callGetTakerAmount
يعمل كلما تمت محاولة التعبئة وإما makingAmount
أو ال takingAmount
المعلمات هي صفر ، على التوالي ، أو إذا كان makingAmount
القيمة أكبر من remainingMakerAmount
.
• _callGetMakerAmount
و _callGetTakerAmount
ستؤدي المكالمات إلى التراجع إذا لم يتم إنشاء الأمر باستخدام getMakerAmount
or getTakerAmount
المعلمات ، على التوالي ، ويتم تنفيذ تعبئة جزئية.
An تعليق مضمن جنبًا إلى جنب _callGetMakerAmount
و تعليق مضمن جنبًا إلى جنب _callGetTakerAmount
الادعاء بأنه "مسموح فقط بالملء الكامل" إذا لم يتم إنشاء الأمر باستخدام getMakerAmount
or getTakerAmount
المعلمات.
ومع ذلك ، هناك مسارات رمز لا ينطبق عليها ذلك ، لأن هذه المسارات لا تتحقق من ملف length
على حد سواء getMakerAmount
و getTakerAmount
المعلمات.
على وجه التحديد، عندما taker
يحدد أ takerAmount
قيمة لأمر يحتوي فقط على getMakerAmount
، ما لم تكن هذه الدعوة إلى getMakerAmount
بإرجاع مبلغ أكبر من remainingMakerAmount
، يمكن تنفيذ التعبئة الجزئية بما يتعارض مع الوثائق المضمنة.
هذا يترك القصد من مسارات الشفرة هذه غير واضح. إذا كان هذا هو السلوك المتوقع ، ففكر في تعديل الوثائق المضمنة بحيث تكون أكثر وضوحًا. إذا كان هذا سلوكًا غير مقصود ، ففكر دائمًا في التحقق من أطوال كل من getMakerAmount
و getTakerAmount
المعلمات في وقت واحد بحيث يعزز التنفيذ السلوك الموصوف في الوثائق المضمنة.
تحديث: الثابتة في طلب سحب # 79.
[L14] استخدام مكالمات Chainlink الموقوفة
• ChainlinkCalculator
العقد مخصص للاستخدام للاستعلام عن Chainlink oracles. يفعل ذلك عن طريق إجراء مكالمات إلى latestTimestamp
و latestAnswer
أساليب، كلاهما تم إهماله. في الواقع ، لم تعد الأساليب موجودة في API لمجمعي Chainlink اعتبارًا من الإصدار الثالث.
لتجنب حالات عدم التوافق المحتملة في المستقبل مع Chainlink oracles ، ضع في اعتبارك استخدام latestRoundData
طريقة بدلا من ذلك.
تحديث: الثابتة في طلب سحب # 67.
ملاحظات ومعلومات إضافية
[N01] عدم استيراد واجهات
• AggregatorInterface
يبدو أن الواجهة عبارة عن مجموعة فرعية من التعليمات البرمجية المنسوخة من ChainLink
مستودع الشفرة العامة. يتم تضمين الواجهة الكاملة في ChainLink
حزمة npm العقد الخاصة بـ.
عندما يكون ذلك ممكنًا ، لتقليل احتمالية عدم تطابق الواجهة والمشكلات الناتجة ، بدلاً من إعادة تعريف و / أو إعادة كتابة واجهات مشروع آخر ، ضع في اعتبارك استخدام الواجهات المثبتة عبر حزم npm الرسمية بدلاً من ذلك.
تحديث: الثابتة في طلب سحب # 66.
[N02] تبعيات مشروع مهملة
أثناء تثبيت ملف تبعيات المشروع، يحذر NPM من أن إحدى الحزم المثبتة ، Highlight
، "لن يتم دعمها أو تلقي تحديثات أمنية في المستقبل".
على الرغم من أنه من غير المحتمل أن تتسبب هذه الحزمة في مخاطر أمنية ، ففكر في ترقية التبعية التي تستخدم هذه الحزمة إلى إصدار تم الحفاظ عليه.
تحديث: الثابتة في طلب سحب # 64.
[N03] المكالمات الخارجية لعرض الأساليب ليست مكالمات ثابتة
خلال معظم قاعدة الكود ، يقوم البروتوكول صراحة بإجراء مكالمات خارجية عبر OpenZeppelin functionStaticCall
طريقة لتقييد إمكانية حدوث تغييرات في الحالة عندما تكون غير متوقعة أو غير مرغوب فيها. ومع ذلك ، في ChainlinkCalculator
العقد ، على الرغم من نية إجراء مكالمات خارجية فقط إلى view
الطرق الموجودة على Chainlink oracles ، والمكالمات الخارجية في ملف singlePrice
و doublePrice
لا يتم إجراء وظائف عبر صريح staticcall
s.
على الرغم من أننا لم نحدد أي مخاوف أمنية فورية ناجمة عن ذلك ، لتقليل سطح الهجوم وتحسين التناسق وتوضيح النية ، فكر في استخدام صريح staticcall
s لجميع المكالمات الخارجية إلى view
وظائف في ChainlinkCalculator
العقد.
تحديث: لم تحل. يقول الفريق 1 بوصة:
نعتقد أن المضاعفات النحوية تبطل التحسينات في الاتساق.
[N04] عدم الفشل مبكرًا مع الطلبات غير الصالحة
في مجلة OrderMixin
العقد fillOrderTo
تتعامل الوظيفة مع الحالة الخاصة عندما لم يتم تقديم طلب سابقًا (remainingMakerAmount == 0
) ، لكنه لا يعالج الشرط صراحة عندما يصبح الأمر غير صالح (remainingMakerAmount == 1
).
في السيناريو الأخير ، ستعود الوظيفة في النهاية ، ولكن فقط بعد حرق كميات غير تافهة من الغاز. لتوضيح النية وزيادة إمكانية القراءة وتقليل استخدام الغاز ، ضع في اعتبارك بوضوح التعامل مع سيناريو الطلب غير الصحيح في بداية الوظيفة.
تحديث: الثابتة في طلب سحب # 68.
[N05] لم يتم تحديد عقود المساعد كمجردة
في Solidity ، الكلمة الأساسية abstract
تُستخدم للعقود التي لا تعد عقودًا وظيفية في حد ذاتها ، أو لا يُقصد استخدامها على هذا النحو. في حين أن، abstract
يتم توريث العقود بواسطة عقود أخرى في النظام لإنشاء عقود وظيفية قائمة بذاتها.
في جميع أنحاء قاعدة الشفرة ، توجد أمثلة على عقود المساعدة التي لم يتم تمييزها على أنها مجردة ، على الرغم من حقيقة أنه لا يُقصد نشرها بمفردها. على سبيل المثال ، ملف AmountCalculator
, ChainlinkCalculator
, ImmutableOwner
, NonceManager
و PredicateHelper
تتكون جميع العقود من مجموعة أساسية من الوظائف التي يُقصد استخدامها عن طريق وراثة العقود.
ضع في اعتبارك تعليم عقود المساعد على أنها abstract
للإشارة بوضوح إلى أنها مصممة فقط لإضافة وظائف للعقود التي ترثها.
تحديث: لم تحل. يقول الفريق 1 بوصة:
يمكن نشر هؤلاء المساعدين بشكل منفصل. هم موروثون فقط لتوفير الغاز.
[N06] ترتيب دالة غير متسق
في جميع أنحاء مصدر البرنامج ، يتبع ترتيب الوظائف بشكل عام الطلب الموصى به في دليل نمط الصلابة، الذي: constructor
, fallback
, external
, public
, internal
, private
.
ومع ذلك ، في OrderMixin
العقد public
checkPredicate
وظيفة تنحرف عن دليل الأسلوب ، شطر external
الوظائف.
لتحسين الوضوح الكلي للمشروع ، ضع في اعتبارك توحيد ترتيب الوظائف في جميع أنحاء قاعدة التعليمات البرمجية ، على النحو الموصى به في دليل نمط Solidity.
تحديث: الثابتة في طلب سحب # 69.
[N07] تدفق تعبئة أمر غير متسق
• OrderMixin
و RFQOrderMixin
يتعامل كلا العقد مع ملء الطلبات الموقعة ، لكن تدفق الأمر العام بين العقدين غير متسق.
OrderMixin
الصورة fillOrderTo
تتبع الوظيفة هذا التدفق العام (رمز زائف):
if ((takingAmount == 0) == (makingAmount == 0))
else if (takingAmount == 0)
else (handle makingAmount == 0) THEN swapTokens
بينما RFQOrderMixin
مماثل fillOrderRFQTo
تتبع الوظيفة هذا التدفق (رمز زائف):
if (takingAmount == 0 && makingAmount == 0)
else if (takingAmount == 0)
else if (makingAmount == 0)
else revert THEN swapTokens
لا توجد رؤى من الوثائق حول سبب اختلاف الشرط الأول في كل من هاتين الوظيفتين ، أو لماذا takingAmount
و makingAmount
لا يمكن أن يكون كلاهما صفرًا في الوظيفة الأخيرة. أيضا ، فإن الحالة التي يكون فيها كلا من makingAmount
و takingAmount
المقدمة أسهل بكثير في التفكير في fillOrderRFQTo
وظيفة ، حيث تم التعامل معها بوضوح في المباراة النهائية else
منع.
لتوضيح النية وزيادة قابلية القراءة الكلية للكود ، ضع في اعتبارك إما توحيد تدفق الطلب العام عبر هذين العقدين ، أو التوثيق الصريح لسبب وجود الاختلافات.
تحديث: لم تحل. يقول الفريق 1 بوصة:
هذا بسبب وظائف التسعير المخصصة في أوامر الحد. حيث
getMakerAmount
من المحتمل أن تختلف اختلافًا جوهريًا عنgetTakerAmount
، اعتقدنا أنه من الأفضل عدم جعل الخيار الافتراضي للمتلقي لأنه من المحتمل أن يربكهم في الحالات التي تكون فيها هذه القيم مختلفة.
[N08] رسائل الخطأ غير منسقة أو مضللة
في جميع أنحاء قاعدة البيانات ، فإن ملف require
و revert
تم العثور على رسائل الخطأ ، التي تهدف إلى إعلام المستخدمين بالظروف الخاصة التي تتسبب في فشل المعاملة ، بتنسيق غير متسق.
على سبيل المثال ، كل واحدة من رسائل الخطأ على الأسطر 85 من OrderMixin.sol
, 16 من ERC721ProxySafe.sol
و 26 من Permitable.sol
تستخدم أسلوبًا مختلفًا.
بالإضافة إلى ذلك ، بعض رسائل الخطأ مضللة:
تهدف رسائل الخطأ إلى إخطار المستخدمين بشروط الفشل ، لذلك يجب عليهم تقديم معلومات كافية بحيث يمكن إجراء التصحيحات المناسبة للتفاعل مع النظام. تؤدي رسائل الخطأ غير المعلوماتية إلى إلحاق ضرر كبير بتجربة المستخدم الإجمالية ، وبالتالي تقليل جودة النظام. علاوة على ذلك ، يمكن أن تؤدي رسائل الخطأ ذات التنسيق غير المتسق إلى حدوث ارتباك لا داعي له. لذلك ، ضع في اعتبارك مراجعة قاعدة التعليمات البرمجية بالكامل للتأكد من كل ملف require
و revert
يحتوي البيان على رسالة خطأ منسقة ودقيقة وغنية بالمعلومات وسهلة الاستخدام بشكل متسق.
تحديث: ثابت جزئيًا في طلب سحب # 81.
[N09] استخدام غير متسق لمتغيرات الإرجاع المسماة
يوجد استخدام غير متسق لمتغيرات الإرجاع المسماة في ملف OrderMixin
اتفافية. بعض الوظائف إرجاع المتغيرات المسماة، الآخرين إرجاع القيم الصريحة، و اخرين أعلن عن متغير عودة مسمى ولكن تجاوزه مع بيان عودة صريح.
ضع في اعتبارك اعتماد نهج ثابت لإرجاع القيم في جميع أنحاء قاعدة التعليمات البرمجية عن طريق إزالة جميع متغيرات الإرجاع المسماة ، والإعلان عنها صراحةً كمتغيرات محلية ، وإضافة عبارات الإرجاع الضرورية عند الاقتضاء. سيؤدي ذلك إلى تحسين كلاً من الوضوح وسهولة قراءة الكود ، وقد يساعد أيضًا في تقليل الانحدار أثناء عمليات إعادة بناء الكود في المستقبل.
تحديث: الثابتة في طلب سحب # 73.
[N10] حساب تجزئة الطلب ليس مفتوحًا لواجهة برمجة التطبيقات
• external
وظائف remaining
, remainingRaw
و remainingsRaw
يتوقع الجميع تجزئة ترتيب لعملية ناجحة.
ومع ذلك ، وظيفة المساعد _hash
، التي تُرجع تجزئة الطلب ، لديها private
الرؤية. هذا يعني أنه سيتعين على المستخدمين حزم أجزاء من الطلبات وسلاسل المجال يدويًا من أجل الحصول على تجزئة الطلب.
لتجنب احتمال حدوث أخطاء عند حساب تجزئات الطلب ولتوفير طريقة للمستخدمين لإنشاء التجزئة الخاصة بالطلب ، ضع في اعتبارك توسيع نطاق رؤية _hash
وظيفة ل public
وإعادة هيكلة الاسم إلى hash
لتكون متسقة مع بقية التعليمات البرمجية.
تحديث: الثابتة في طلب سحب # 74.
[N11] التحميل الزائد الدلالي لرسم الخرائط
• _remaining
رسم الخرائط في OrderMixin
العقد مثقل من الناحية اللغوية لتتبع حالة الطلبات والمبلغ المتبقي من الأصول المتاحة لتلك الطلبات.
الدول الثلاث التي يمكن أن تتخذها هي:
0
: ترتيب التجزئة لم يتم رؤيته بعد.1
: الطلب قد تم إلغاؤه أو تنفيذه بالكامل.- اي
uint
أكبر من1
: المتبقيmakerAmount
متاح ليتم ملؤه على أمر زائد1
.
يتطلب هذا التحميل الزائد الدلالي التفاف وفك هذه القيمة أثناء lookup
, cancellation
, initialization
و storage
.
قد يكون التحميل الزائد الدلالي والمنطق الضروري لتمكينه عرضة للخطأ ويمكن أن يجعل من الصعب فهم قاعدة الكود وتفسيرها ، وقد يفتح الباب أيضًا للانحدار في التحديثات المستقبلية للرمز.
لتحسين قابلية قراءة الكود ، ضع في اعتبارك تتبع حالة إكمال الطلبات في تعيين منفصل.
تحديث: لم تحل. أشار فريق 1 بوصة إلى أن الإصلاح سيزيد من تكاليف الغاز لـ fillOrder
وظيفة.
[N12] الأوامر ذات التصاريح تسمح باستدعاء العقود التعسفية
• OrderMixin
العقد يرث Permitable
عقد للسماح بملء أمر المعاملة الواحدة بالأصول التي تقبل ذلك permit
مكالمات لتعديل البدلات.
ومع ذلك، فإن المكالمات إلى Permitable
عقد لا تتحقق مما إذا كان الهدف أصلاً مسموحًا به ولا حتى إذا كان أصلًا ، مما قد يسمح لمستخدم ضار بتمرير عنوان عقد تعسفي يمكن أن ينفذ مكالمة أخرى قبل اكتمال ملء الطلب.
على الرغم من أن العقد محمي ضد العودةيوصى دائمًا بتقليل سطح الهجوم ومنع استدعاء العقود الخارجية أثناء التنفيذ. ضع في اعتبارك إما تقييد الأصل المتضمن في التصريح على الأصول المتضمنة في الأمر أو إلى القائمة البيضاء للأصول الخاصة بالبروتوكول.
تحديث: لم تحل. يقول الفريق 1 بوصة:
OrderMixin
لا يحتوي في الواقع على معلومات حول الرموز الفعلية مثلmakerAsset
وtakerAsset
في بعض الأحيان وكلاء أو عقود وسيطة أخرى ويتم تخزين المعلومات حول الرموز الفعلية في بعض وحدات البايت التعسفية. لذلك لا توجد طريقة قابلة للتطبيق لتقييد أي أصلpermit
يسمى على.
[رقم 13] solhint
لم يتم إعادة تمكينه أبدًا
في جميع أنحاء قاعدة البيانات ، يوجد زوجان من solhint-disable
البيانات ، وتحديدا تلك على الإنترنت 23 وعلى الإنترنت 41 of RevertReasonParser.sol
، التي لم يتم إنهاؤها بـ solhint-enable
.
لصالح الصراحة وأن تكون تقييدية قدر الإمكان عند التعطيل solhint
، فكر في استخدام solhint-disable-line
or solhint-disable-next-line
بدلا من ذلك ، على غرار الخط 16 من نفس الملف.
تحديث: الثابتة في طلب سحب # 72.
[N14] الأخطاء المطبعية
يحتوي مصدر البرنامج على الأخطاء المطبعية التالية:
بالإضافة إلى المشروع README
(خارج نطاق هذا التدقيق) يحتوي على الأخطاء المطبعية التالية:
ضع في اعتبارك تصحيح هذه الأخطاء المطبعية لتحسين إمكانية قراءة الكود.
تحديث: الثابتة في طلب سحب # 71 و طلب سحب # 77.
[N15] استخدام uint
بدلا من uint256
لتفضيل الوضوح ، جميع حالات uint
يجب إعلانها على أنها uint256
. على وجه الخصوص ، تلك الموجودة في for
حلقات على خطوط 98 و 119 of OrderMixin.sol
والخطوط 16 و 30 of PredicateHelper.sol
.
تحديث: الثابتة في طلب سحب # 70.
استنتاجات
تم العثور على 3 قضايا شديدة الخطورة. تم اقتراح بعض التغييرات لاتباع أفضل الممارسات وتقليل سطح الهجوم المحتمل.
- &
- 7
- من نحن
- الوصول
- وفقا
- حسابي
- في
- عمل
- الإجراءات
- إضافي
- العنوان
- مميزات
- الكل
- السماح
- سابقا
- بالرغم ان
- المبالغ
- تحليل
- API
- نهج
- الحجج
- حول
- الأصول
- ممتلكات
- التدقيق
- الخلفية
- البداية
- يجري
- أفضل
- أفضل الممارسات
- قطعة
- البوتات
- نساعدك في بناء
- دعوة
- يهمني
- الحالات
- سبب
- سلسلة ربط
- تغيير
- تدقيق
- الشيكات
- الكود
- مجمع
- حالة
- ارتباك
- إنشاء
- يحتوي
- عقد
- عقود
- التصحيحات
- التكاليف
- استطاع
- زوجان
- الخالق
- العملة
- حالياًّ
- البيانات
- صفقة
- الحرمان من الخدمة
- نشر
- تصميم
- المطورين
- التطوير التجاري
- فعل
- اختلف
- مختلف
- نطاق
- مضاعفة
- ديناميكي
- في وقت مبكر
- حافة
- شجع
- خاصة
- ETH
- الحدث/الفعالية
- أحداث
- كل شىء
- مثال
- تبادل
- متوقع
- الخبره في مجال الغطس
- استغلال
- المميزات
- مجال
- أخيرا
- الاسم الأول
- حل
- مرونة
- تدفق
- اتباع
- وجدت
- بالإضافة إلى
- وظيفة
- أموال
- مستقبل
- لعبة
- GAS
- العلاجات العامة
- إعطاء
- عظيم
- توجيه
- معالجة
- مزيج
- الثرم
- وجود
- مساعدة
- مرتفع
- جدا
- كيفية
- HTTPS
- مهجنة
- تحديد
- التأثير
- تنفيذ
- أهمية
- استيراد
- شامل
- بما فيه
- القيمة الاسمية
- زيادة
- info
- معلومات
- البنية التحتية
- رؤى
- نية
- مصلحة
- السطح البيني
- المشاركة
- مسائل
- IT
- كبير
- أكبر
- قيادة
- قيادة
- المكتبة
- محدود
- خط
- سيولة
- المدرج
- قوائم
- محلي
- بدا
- بحث
- رائد
- صانع
- القيام ب
- تجارة
- Mempool
- مرآة
- نموذج
- أكثر
- الاكثر شهره
- أي
- مزايا جديدة
- غير قابلة للاستبدال
- الرموز غير قابلة للاستبدال
- رسمي
- جاكيت
- عمليات
- خيار
- أوراكل
- طلب
- الطلبات
- أخرى
- كاتوا ديلز
- نمط
- أكثر الاستفسارات
- يقدم
- منع
- السعر
- التسعير
- خاص
- عملية المعالجة
- تنفيذ المشاريع
- مشروع ناجح
- الحماية
- بروتوكول
- تزود
- ويوفر
- الوكيل
- جمهور
- نشر
- شراء
- جودة
- رفع
- واقع
- تخفيض
- اعتماد
- تقرير
- التقارير
- مستودع
- المتطلبات الأساسية
- REST
- النتائج
- عائدات
- مراجعة
- المخاطرة
- جولات
- يجري
- الإستراحة
- أمن
- خدمات
- طقم
- شاركت
- مشاركة
- نقل
- مماثل
- الاشارات
- صغير
- سمارت
- العقود الذكية
- So
- صلابة
- البريد المزعج
- على وجه التحديد
- الإنفاق
- بقعة
- بداية
- الولايه او المحافظه
- ملخص الحساب
- المحافظة
- الحالة
- تخزين
- نمط
- المقدمة
- تحقيق النجاح
- ناجح
- بنجاح
- الدعم
- مدعومة
- المساحة
- مفاتيح
- نظام
- الهدف
- تجربه بالعربي
- الاختبار
- اختبارات
- عبر
- طوال
- الوقت
- سويا
- رمز
- الرموز
- مسار
- تتبع الشحنة
- صفقة
- الثقة
- فريد من نوعه
- آخر التحديثات
- us
- قابليتها للاستخدام
- USD
- USDT
- المستخدمين
- سهل حياتك
- قيمنا
- المزيد
- رؤية
- انتظر
- ابحث عن
- القائمة البيضاء
- في غضون
- بدون
- للعمل
- قيمة
- صفر