6 كانون الأول، 2021
المُقدّمة
• عد طلب منا فريق العمل مراجعة وتدقيق مجموعة من العقود بهدف نهائي يتمثل في تحسين عقود الحوكمة المشتركة ومنحها مزيدًا من المرونة. نظرنا إلى الكود ونشرنا نتائجنا الآن.
نبذة عن النظام
يعتمد النظام على ثلاثة عقود رئيسية:
- A
SafeGuard
نموذج العقد. هذا العقد هوadmin
منTimelock
العقد ، ويضيف طبقة من الأدوار المعيارية فوقTimelock
أفعال. يحدد هذا العقد العديد من الأدوار التي لها مسؤولية منفصلة وإمكانية الوصول إلى حالة الاقتراح (قائمة الانتظار والإلغاء والتنفيذ). - A
SafeGuardFactory
ينشر عقد جديدSafeGuard
والجديد المقابلTimelock
اتفافية. ثم يقوم بتعيين عنوان timelock داخل ملفSafeGuard
العقد ويسجل الجديدSafeGuard
فيRegistry
. - A
Registry
الذي يحمل قائمة منتشرةSafeGuards
مع ما يقابلهم رقم الإصدار.
• SafeGuardFactory
سوف تفرخ أساسًا ملف جديد SafeGuard
متى تم استدعاؤها. أ SafeGuard
هو التفاف حول Timelock
عمليات يضيف أدوارًا مختلفة ومنفصلة لكل إجراء. يتم تنظيم الأدوار من خلال استخدام OpenZeppelin AccessControlEnumerable
يمنح العقد مزيدًا من المرونة والتوافق مع محافظ multisig وحالات استخدام الأعمال حيث يمكن أن يكون للعديد من العناوين نفس الدور المشترك.
تحديث: في مجلة رقم العلاقات العامة 10، قرر فريق Tally إزالة Registry
اتفافية. يتم الآن تخزين قائمة الضمانات التي تم نشرها داخل ملف SafeGuardFactory
العقد ، في safeGuards
مجموعة لا تعد ولا تحصى ، جنبًا إلى جنب مع نسختها المخزنة في ملف safeGuardVersion
رسم الخرائط.
الأدوار
• SafeGuard
يحدد العقد الأدوار التالية:
تحديث: • CREATOR_ROLE
تمت إزالة الدور كإصلاح للمشكلة L02.
مجال
دققنا الالتزام b2c63a9dfc4090be13320d999e7c6c1d842625d3
ل safeguard
مخزن. في النطاق هي العقود الذكية في contracts
الدليل. ومع ذلك ، فإن mocks
تم اعتبار الدليل خارج النطاق.
الافتراضات
لا يُقصد من النظام أن يكون قابلاً للترقية. ال Registry
تم تعيين العنوان في المنشئ ل SafeGuardFactory
وكل SafeGuard
يمكن أن يكون Timelock
تعيين مرة واحدة فقط. هذا يعني أنه إذا كان ملف Registry
تم نشر ملف SafeGuardFactory
يجب نشرها أيضًا. إذا كان Timelock
تغييرات التنفيذ القديمة SafeGuard
سيصبح قادمًا ، وسيتعين نشر أجهزة جديدة.
علاوة على ذلك ، يعتمد النظام بشكل كبير على تنفيذ أ Timelock
عقد التي تم اعتبارها خارج نطاق هذا التدقيق. لم ينته الفريق بعد من تنفيذ Timelock
اتفافية. في وقت كتابة هذا التقرير ، تنفيذ الكمبوند لـ timelock يتم استخدامه في المشروع.
تم تدقيق قاعدة البيانات من قبل مدققين خلال أسبوع واحد ، وهنا نقدم النتائج التي توصلنا إليها.
شدة حرجة
لا شيء.
خطورة شديدة
[H01] يمكن قفل ETH داخل Timelock
عقد
• Tally
فريق أسس تطبيقاتهم في الأصل على أرض الواقع GovernorBravoDelegate
عقد مركب.
خلال مسار هذا التدقيق ، فإن Tally
اكتشف الفريق قيد في محافظ الكمبوند حيث أرسل ETH مباشرة إلى Timelock
غير متاح للاستخدام من خلال مقترحات الحوكمة ، وعلى الرغم من أنه ليس متوقفًا بشكل دائم ، إلا أنه يتطلب حلاً بديلاً مفصلاً ليتم استرجاعه.
وذلك لأن تنفيذ المحافظ يتطلب إرفاق كل قيمة الاقتراح على أنها msg.value
من خلال الحساب الذي أدى إلى التنفيذ ، وليس باستخدام بأي شكل من الأشكال Timelock
صناديق ETH.
• تم تحديد نفس المشكلة لاحقًا في SafeGuard
التنفيذ والفريق على علم بالمشكلة وهو بصدد إصلاحها.
أثناء إصلاح المشكلة ، ضع في اعتبارك استخدام النهج التي اعتمدتها مكتبة OpenZeppelin لنفس المشكلة.
تحديث: ثابت في الالتزام 7337db227edda83533be586135d96ddac4f5bf29
.
[H02] يمكن تجميد SafeGuardFactory
• Registry
العقد يهدف إلى تتبع جميع SafeGuards
أن SafeGuardFactory
ينتج عنه. لها الخارجية register
وظيفة الذي يستخدم لهذا الغرض.
وفي الوقت نفسه، SafeGuardFactory
لديه ، في منشئه ، التخصيص المحلي registry
قيمة لقيمة الإدخال. لا توجد إمكانية لتغيير قيمة registry
لهذا السبب ، إذا كان ملفًا جديدًا Registry
يتم نشرها ، يجب نشر مصنع جديد أيضًا.
• SafeGuardFactory
لديه createSafeGuard
وظيفة، المسؤول الأول نشر ملف SafeGuard
، ثم جديد Timelock
مع عنوان SafeGuard
as admin
, ثم ضبط timelock
متغير SafeGuard
العقد وأخيرا تسجيل SafeGuard
في التسجيل.
القضية هي أن أي دعوة ل createSafeGuard
يمكن إجبارها على الفشل من قبل مهاجم يمكنه تسجيل العنوان الحتمي للجديد مباشرة SafeGuard
قبل إنشائها. كلما عقد يخلق new
مثل، يتم زيادة nonce الخاص به ، ويمكن تحديد عنوان المكان الذي سيتم فيه نشر النسخة الجديدة من العقد من خلال عنوان العقد الأصلي و nonce الخاص به. لذلك ، يمكن للمهاجم إجراء حساب مسبق للعديد من العناوين التي تحتوي على عناوين جديدة SafeGuards
سيتم نشرها وتسجيل تلك العناوين في Registry
عن طريق استدعاء register
وظيفة. هذا من شأنه أن يؤدي إلى المكالمات إلى createSafeGuard
إلى العودة منذ Registry
يحتوي بالفعل على العنوان.
لتجنب قيام جهات خارجية بالدعوة علنًا لـ Register
العقد ، ففكر في تقييد الوصول إلى register
وظيفة لقبول المكالمات حصريًا بواسطة SafeGuardFactory
.
تحديث: الثابتة في رقم العلاقات العامة 10. قام فريق Tally بإزالة Registry
العقد.
شدة متوسطة
لا شيء.
شدة منخفضة
[L01] علق كود
• Registry
يشمل العقد سطر من التعليمات البرمجية المعلق عليها. لتحسين إمكانية القراءة ، ضع في اعتبارك إزالته من مصدر البرنامج.
تحديث: الثابتة في رقم العلاقات العامة 10 والالتزام 7fd27df16fc879d990d36a167a0b6e719e578558
.
[L02] يمكن لمسؤول SafeGuard تعيين دور المنشئ لأي عنوان
• SafeGuard
عقد يحدد دور CREATOR_ROLE
والتي ، كما يوحي الاسم ، مخصصة لمنشئ الحماية.
ومع ذلك ، من خلال التذرع ال grantRole
وظيفة من AccessControlEnumerable
عقد في مكتبة عقود OpenZeppelin ، يمكن للمسؤول منح هذا الدور لأي عنوان. قد يتسبب هذا في حدوث ارتباك لأن ملف خالق SafeGuard
يمكن أن يكون فقط SafeGuardFactory
.
خلال قاعدة التعليمات البرمجية ، تم استخدام هذا الدور فقط لتقييد المستخدمين من التفاعل مع setTimelock
وظيفة ل SafeGuard
اتفافية. حسب التصميم ، يضمن النظام ذلك setTimelock
وظيفة يمكن الاتصال به مرة واحدة فقط، من في حدود SafeGuardFactory
عقد.
ضع في اعتبارك إزالة ملف CREATOR_ROLE
دور من SafeGuard
العقد واستخدام onlyOwner
تغيير في ال setTimelock
وظيفة.
تحديث: الثابتة في رقم العلاقات العامة 10.
[L03] تعريف واجهة غير صحيحة وتنفيذها
• ISafeGuard
الواجهة لا تعرف queueTransactionWithDescription
وظيفة نفذت في SafeGuard
العقد ، وفي الوقت نفسه ، يحدد __abdicate و __queueSetTimelockPendingAdmin و __executeSetTimelockPendingAdmin وظائف ولكن لم يتم تنفيذها.
لتحسين الدقة والاتساق في قاعدة التعليمات البرمجية ، ضع في اعتبارك إعادة هيكلة ملف ISafeGuard
واجهة لتتناسب تمامًا مع ملف SafeGuard
التنفيذ.
تحديث: ثابت في الالتزام 7fd27df16fc879d990d36a167a0b6e719e578558
.
[L04] سلاسل مستندات مفقودة
تفتقر بعض العقود والوظائف في قاعدة الرمز إلى التوثيق. علي سبيل المثال، بعض الوظائف في ال SafeGuard
العقد.
بالإضافة إلى ذلك ، تستخدم بعض السلاسل لغة غير رسمية ، مثل اللغة فوق setTimelock
وظيفة في SafeGuard
عقد.
هذا يعيق فهم المراجعين لنية الكود ، وهو أمر أساسي لتقييم ليس فقط الأمان بشكل صحيح ولكن أيضًا للصحة. بالإضافة إلى ذلك ، تعمل سلاسل المستندات على تحسين إمكانية القراءة وتسهيل الصيانة. يجب أن تشرح صراحة الغرض أو النية من الوظائف ، والسيناريوهات التي يمكن أن تفشل بموجبها ، والأدوار المسموح لها باستدعاءها ، والقيم التي تم إرجاعها والأحداث المنبعثة.
ضع في اعتبارك التوثيق الشامل لجميع الوظائف (ومعاييرها) التي تشكل جزءًا من واجهة برمجة التطبيقات العامة للعقود. يجب توثيق الوظائف التي تنفذ وظائف حساسة ، حتى وإن لم تكن عامة ، بشكل واضح أيضًا. عند كتابة السلاسل النصية ، ضع في اعتبارك اتباع تنسيق المواصفات الطبيعية لإيثريوم (نات سبيك).
تحديث: ثابت جزئيًا في رقم العلاقات العامة 10. تمت إضافة سلاسل الوثائق المناسبة إلى وظائف مختلفة في جميع أنحاء قاعدة الكود. ومع ذلك ، بالإضافة إلى التغييرات الحالية ، ضع في اعتبارك إجراء التغييرات التالية:
- أضف
description
كما@param
في docstring أعلاهqueueTransactionWithDescription
وظيفة - أضف
@param
في ال docstring فوقcreateSafeGuard
وظيفة فيSafeGuardFactory
عقد - أضف
@return
في الوثائق فوق الوظائف فيSafeGuardFactory
العقد.
[L05] كود عديم الفائدة أو مكرر
توجد أماكن في قاعدة الشفرة حيث يتم تكرار الشفرة أو عدم الحاجة إليها. بعض الأمثلة هي:
- الأسطر 29-32 ل
Registry
العقد غير مجدية ، لأن_add
وظيفة منEnumerableSet
عقد يجري بالفعل هذه الفحوصات مقابل القيم التي تم تعيينها بالفعل. - خطوط 62, 67, 73 و 78 ل
SafeGuard
العقد يكرر نفس العملية بالضبط. ضع في اعتبارك تغليفها في وظيفة داخلية لتجنب تكرار التعليمات البرمجية. - الأسطر 62-63 و 67-68 of
SafeGuard
تتكرر. ضع في اعتبارك تغليفها في وظيفة داخلية واحدة. - استخدام
gasleft
لتحديد كمية الغاز التي يجب إعادة توجيهها في استدعاء الوظيفةexecuteTransaction
غير ضروري. هذا لأنه ، في هذه المرحلة من التنفيذ ، سيتم استخدام الغاز المتبقي بأكمله لمواصلة التنفيذ. إذا لم يكن هذا لغرض التوضيح ، ففكر في إزالةgas
المعلمة من المكالمة.
ضع في اعتبارك تطبيق الثابت المقترح لإنتاج كود أنظف وتحسين التناسق والنمطية على قاعدة الكود.
تحديث: الثابتة في رقم العلاقات العامة 10 والالتزام 7fd27df16fc879d990d36a167a0b6e719e578558
.
ملاحظات ومعلومات إضافية
[N01] أسلوب غير متسق
هناك بعض الأماكن في قاعدة الكود ، حيث تؤثر الاختلافات في الأسلوب على قابلية القراءة ، مما يجعل فهم الكود أكثر صعوبة. بعض الأمثلة هي:
- •
Registry
يستخدم العقد أنماطًا مختلفة للوثائق في العقد بأكمله. - •
SafeGuard
العقد ينبعث منها حدث عندماqueueTransactionWithDescription
يتم استدعاؤه ولكن لم يتم إصدار أي أحداث في وظائف أخرى تتعامل مع المعاملات. - في مجلة
SafeGuard
العقد ، في بعض الأحيان قيمنا يستخدم كمعامل مسمى وأحيانًا _القيمة .
مع الأخذ في الاعتبار القيمة التي يضيفها نمط الترميز المتسق إلى سهولة قراءة المشروع ، ضع في اعتبارك فرض نمط ترميز قياسي بمساعدة أدوات linter ، مثل Solhint.
تحديث: الثابتة في رقم العلاقات العامة 10 والالتزام 7fd27df16fc879d990d36a167a0b6e719e578558
.
[N02] ترخيص مفقود
تفتقد العقود التالية ضمن قاعدة التعليمات البرمجية معرف ترخيص SPDX.
لإسكات تحذيرات المحول البرمجي وزيادة التناسق عبر قاعدة التعليمات البرمجية ، ضع في اعتبارك إضافة معرف ترخيص. أثناء القيام بذلك ، ضع في اعتبارك الإشارة إلى spdx.dev المبادئ التوجيهية.
تحديث: الثابتة في رقم العلاقات العامة 10 والالتزام 7fd27df16fc879d990d36a167a0b6e719e578558
.
[N03] لم يتم تثبيت تبعية عقد OpenZeppelin
لمنع السلوكيات غير المتوقعة في حالة إصدار تغييرات متقطعة في التحديثات المستقبلية لـ مكتبة عقود OpenZeppelin، ضع في اعتبارك تثبيت إصدار هذه التبعية في ملف package.json ملف.
تحديث: الثابتة في رقم العلاقات العامة 10.
[N04] إصدار مترجم Solidity غير مثبت
في جميع أنحاء قاعدة التعليمات البرمجية ، ضع في اعتبارك تثبيت إصدار مترجم الصلابة إلى أحدث إصدار مستقر. يجب أن يساعد هذا في منع إدخال أخطاء غير متوقعة بسبب الإصدارات المستقبلية غير المتوافقة. لاختيار إصدار معين ، يجب على المطورين مراعاة ميزات المترجم التي يحتاجها المشروع و قائمة البق المعروفة المرتبطة بكل إصدار مترجم Solidity.
تحديث: الثابتة في رقم العلاقات العامة 10.
[N05] خطأ مطبعي
في حالات مختلفة في جميع أنحاء قاعدة التعليمات البرمجية ، الكلمة role
خطأ إملائي كـ rol
. أحد الأمثلة على ذلك هو في في docstring داخل constructor
ل SafeGuard
عقد.
ضع في اعتبارك تصحيح هذه الأخطاء المطبعية لتحسين إمكانية قراءة الكود.
تحديث: ثابت جزئيًا في رقم العلاقات العامة 10. أثناء تهجئة role
تم تصحيح التعليق "تعيين دور المسؤول على عنوان مسؤول محدد" يجب أن يكون "تعيين دور المسؤول إلى عنوان مشرف محدد". بالإضافة إلى ذلك ، يوجد خطأ إملائي في "تنفيذ" في الملف SafeGuard
عقد على خط 69, خط 82, خط 96 و خط 110 و "متوفر" بها خطأ إملائي في خط 70, خط 83, خط 97, خط 111. أيضًا ، ضع في اعتبارك استبدال الكلمات غير الرسمية مثل "سوف" in SafeGuard
التعاقد مع بدائل رسمية مثل "الذهاب إلى".
[N06] التصريح باسم uint256
هناك العديد من التكرارات في قاعدة الشفرة حيث تم التصريح عن المتغيرات uint
نوع البيانات بدلا من uint256
. على سبيل المثال ، فإن eta
متغير في QueueTransactionWithDescription
حدث ل SafeGuard
العقد.
لتفضيل الوضوح ، جميع حالات uint
يجب إعلانها على أنها uint256
.
تحديث: الثابتة في رقم العلاقات العامة 10 والالتزام 7fd27df16fc879d990d36a167a0b6e719e578558
.
[N07] استيراد غير مستخدم
• SafeGuard
العقد واردات console
العقد ولكن لا تستخدمه أبدًا.
لتحسين إمكانية قراءة الشفرة ، ضع في الاعتبار إزالة أي عمليات استيراد غير مستخدمة.
تحديث: الثابتة في رقم العلاقات العامة 10.
استنتاجات
تم العثور على ثغرة واحدة عالية والعديد من الثغرات الثانوية الأخرى واقتُرحت التوصيات والإصلاحات.
- &
- الوصول
- حسابي
- في
- اكشن
- الإجراءات
- إضافي
- العنوان
- مشرف
- الكل
- سابقا
- بالرغم ان
- API
- نهج
- حول
- التدقيق
- في الأساس
- يجري
- البق
- الأعمال
- دعوة
- الحالات
- سبب
- تغيير
- تهمة
- الكود
- البرمجة
- مشترك
- مركب
- ارتباك
- نظر
- يحتوي
- استمر
- عقد
- عقود
- استطاع
- الخالق
- حالياًّ
- البيانات
- تعامل
- تصميم
- المطورين
- مختلف
- اكتشف
- توضيح
- ETH
- الحدث/الفعالية
- أحداث
- مثال
- مصنع
- المميزات
- أخيرا
- الاسم الأول
- حل
- مرونة
- وجدت
- وظيفة
- أموال
- مستقبل
- GAS
- إعطاء
- هدف
- الحكم
- محافظ
- المبادئ التوجيهية
- وجود
- مساعدة
- هنا
- مرتفع
- كيفية
- HTTPS
- نفذت
- القيمة الاسمية
- زيادة
- السطح البيني
- IT
- معروف
- لغة
- آخر
- المكتبة
- حقوق الملكية الفكرية
- خط
- قائمة
- محلي
- مقفل
- بدا
- القيام ب
- مباراة
- وحدات
- Multisig
- أخرى
- يقدم
- عملية المعالجة
- تنفيذ المشاريع
- مقترح
- جمهور
- نشر
- تسجيل جديد
- النشرات
- تقرير
- مستودع
- النتائج
- مراجعة
- أمن
- طقم
- ضبط
- شاركت
- سمارت
- العقود الذكية
- صلابة
- الولايه او المحافظه
- نمط
- نظام
- عبر
- طوال
- الوقت
- أدوات
- مسار
- المعاملات
- آخر التحديثات
- us
- المستخدمين
- قيمنا
- نقاط الضعف
- محافظ
- أسبوع
- من الذى
- في غضون
- كلمات
- جاري الكتابة