ביקורת חוזים חכמה עבור ZenPool

תקציר ניהול

צוות ZenPool יצר קשר עם Sayfer Security על מנת לבצע ביקורת אבטחה מלאה עבור כל החוזים שלהם.

לפני הערכת השירותים הללו קיימנו פגישת פתיחה עם הצוות הטכני של ZenPool וקיבלנו סקירה כללית של המערכת והמטרות להערכה זו.

הביקורת הבאה נמשכה 20 ימי עבודה, כל החוזים על השרשרת נבדקו שורה אחר שורה עם לפחות 2 מבקרים לכל חוזה. בשל מגבלות זמן מצד הלקוח, בדקנו רכיבים מחוץ לרשת ונקטנו בגישה הטובה ביותר כדי לוודא שהם אינם מכילים נקודות תורפה קריטיות.

מצאנו בסך הכל 9 ממצאים, מתוכם 3 סווגו כנקודות תורפה בסיכון "גבוה" ויכולים להיות ניתנים לניצול על ידי תוקפים זדוניים, שעלולים לרוקן לחלוטין את כספי המאגר.

תיעדנו את התהליך שלנו ואת ההצעות שלנו כיצד יש לתקן כל פגיעות. הצוות של ZenPool יישם את התיקונים שתועדו בכל סעיף פגיעות.

סקירת מערכת

ZenPool הוא קוד פתוח, לא משמורת, ופרוטוקול שוק הלוואות.

משתמשים יכולים להפקיד את נכסי הקריפטו שלהם כדי להרוויח ריבית או ללוות אסימונים אחרים כדי לשלם ריבית בשוק של ZenPool. ל-ZenPool יש אסימון משלו בשם ZEN.

ZEN הוא מטבע צף חופשי המגובה באספקת המטבעות היציבה של BUSD האוצר. ניתן להטביע ולצרוב אסימוני ZEN רק על ידי הפרוטוקול, רק בתגובה למחיר הפרוטוקול עושה זאת. כל ZEN נתמך על ידי BUSD אחד לפחות.

אם המחיר של ZEN יורד מתחת ל-1 BUSD, הפרוטוקול קונה ושורף ZEN, ודוחף את המחיר בחזרה ל-1 BUSD.

ZenPool תומך גם באגרות חוב שהן דרך נוספת להגדיל את האוצר שלה. הפרוטוקול מוכר אגרות חוב בתמורה לנכסים שונים, ובתמורה, הקונה מקבל ZEN במחיר מופחת משמעותית. זה מגביר את האוצר ומאפשר ל-ZeenPool לספק תשואות מדהימות ללקוחותיה.

לבסוף, חלק מכל עמלות המוצרים של ZenPool ישמשו כגיבוי נוסף, מה שעלול לספק לאסימון של ZenPool מסלול אינסופי להצבת תגמולים.

ממצאי ביקורת חוזים חכמים

משתמשים יכולים למשוך כספים מיתרות אחרות עד שהבריכה תתרוקן

תיאור

השמיים withdrawFunds הפונקציה מאמתת שהמשתמש יכול למשוך רק את הסכום המקסימלי שביתרה שלו, נבדק לפי הכתובת שלו והושווה מאוחר יותר בתוך מאגר האסימונים.

מאוחר יותר הסכום שנמשך מנוכה מההלוואה המקסימלית הנוכחית של המשתמש במחיר הנוכחי. אם הסכום הכולל של הכספים שהושאלו על ידי המשתמש חורג מההלוואה המקסימלית החדשה, השיטה נכשלת מכיוון שלמשתמש אין עוד מספיק בטחונות כדי לתמוך בעמדת ההלוואה שלו. הדרישה הזו, עם זאת, נבדקת רק אם המשתמש עדיין לא ממונף יתר על המידה:

תוקף יכול להשתמש בפונקציונליות זו כדי לנצל את withdrawFunds פונקציה למשוך יותר מסכום ההלוואה המקסימלי.

הוא יכול לעשות את זה כי getMaxBorrow ייבדק רק אם המשתמש לווה פחות מהסכום המקסימלי המותר לו, במגוון תרחישים התוקף יכול לנצל לרעה את הזרימה כדי לדלג על הספציפי הזה require, המאפשר לו להתקשר ל withdrawFund מספר פעמים עד שהבריכה תתרוקן.

הקלות

לשנות את ה require להתקשר לפני קו 145 כך שזה לא תלוי בהצהרת if. בדרך זו ה require תמיד יבוצע.

הרס עצמי לא בטוח בחוזה פרוקסי

תיאור

כאשר העיקרית ZenPoolManager פרוס, הוא גם פורס חוזים מרובים.

אחד מהם הוא ZenTickets חוזה שברובו מובטח, למעט ה destoryContract פונקציה שאין לה מנגנון ACL כמו שאר הפונקציות:

על ידי ניצול פונקציונליות זו, תוקף לא מאומת יכול לקרוא ל- destoryContract לתפקד ולבחור לאן יועבר ה-ETH של החוזה. קל לנצל זאת מנקודת המבט של התוקף.

הקלות

השתמש onlyOwner שינוי כמו שהוא משמש באיפוס הפונקציות.

פתרון נוסף ספציפי יותר יהיה להשתמש במנגנון ACL מותאם אישית כמו ACL תפקידים של openzeppelin שיאפשר רק לתפקידים ספציפיים לגשת ל- destoryContract פונקציות

מאגרי המשתמש נתונים להתקפות MEV

תיאור

העיקרי ZenPoolUserManager מטפל במאגרי משתמשים מותאמים אישית שנוצרו במערכת. המשתמש יכול להתקשר לפעולות שונות במאגרים אלה אם יש לו את ההרשאות הנכונות לעשות זאת, משתמש יכול גם להעניק גישה באמצעות מנגנון תפקיד המבוסס על חוזי בקרת הגישה של OpenZeppelin

השמיים ZenPoolUserManager בעל מנגנון ACL תקין, אך יחד עם זאת, יש לו 2 פונקציות הנתונות לריצה קדמית/אחורית או כל התקפות MEV.

בעוד שההיגיון העסקי עצמו נכון, ונראה שהוא לא יכול להזיק הרבה מכיוון שיש לו ACL תקין, משתמש עם אותו תפקיד יכול לנצל אותו באמצעות התקפות MEV.

הקלות

השתמש onlyOwner שינוי כמו שהוא משמש באיפוס הפונקציות.

פתרון נוסף ספציפי יותר יהיה להשתמש במנגנון ACL מותאם אישית כמו ACL תפקידים של openzeppelin שיאפשר רק לתפקידים ספציפיים לגשת ל- destoryContract פונקציות.

היעדר הפחתה של האפוטרופוס

תיאור

במהלך הביקורת שלנו ביצענו ניתוח קוד מקורי, והשווינו את בסיס הקוד הנוכחי של הלקוח ל-

פרויקטי קוד פתוח שבהם השתמש הלקוח לפיתוח הקוד. אם גילינו שבוצעו שינויים בקוד הקוד הפתוח, בדקנו אותם לעומק כדי לוודא שהם נעשו בחוכמה.

רוב הלקוחות רואים בפרויקטים של קוד פתוח של צד שלישי מאובטחים מכיוון שהם מגיעים מספקים גדולים (מאגר EG Uniswap). ההנחה הזו נכונה ברובה. עם זאת, באגי אבטחה גדולים מתרחשים כאשר לקוחות מבצעים שינויים קלים בקוד ומניחים ששינויים אלו אינם משפיעים על האבטחה הכוללת של המוצר.

במהלך ניתוח הקוד המקורי שלנו, מצאנו ש-ZenPool משתמש בקוד מפרוטוקול STRIKE על מנת ליצור אסימון ממשל, הקוד המקורי מ-STRIKE repo הוא:

בעוד הקוד ב-ZenGovernorAlpha הוא:

ההצהרה הבאה הוסרה msg.sender == guardian, משמעות הדבר היא שהאפוטרופוס לא יכול לבטל הזמנות אם הסף הגיע. ביטול כוחם של האפוטרופוסים עלול לגרום למצבים מסוכנים מאוד. למשל, אם בוצעה הצעה שנפשרה שמעבירה את כל כספי החוזה לשחקן זדוני (על ידי פריצת מפתח פרטי למשל) מנגנון הגנת הסף מיותר ולא יכול לעזור, כי אין אפוטרופוס שיכול לעצור את ההצעה מלהתרחש.

הקלות

להוסיף msg.sender == guardian אל ה require .

חוסר ב-Reenetrancy Guard

תיאור

פונקציית ההשאלה הבאה אינה מכילה שומר כניסה חוזר וגם לא דפוס אינטראקציות של בדיקות-אפקטים, זה כרגע לא ניתן לניצול מכיוון שהבריכה תומכת רק באסימוני ERC20, אבל אם בעתיד יתווסף ERC-777 חדש, יכול להתרחש ניצול של כניסה חוזרת. לגרום למשוך מלא של כספי החוזה.

בנוסף, הקוד אינו עוקב אחר דפוס ה-checks-effects-interactions. במקרה הספציפי הזה, ה require נבדק לאחר שהלוגיקה העסקית כבר מיושמת, הדבר עלול לגרום לבאגים של כניסה חוזרת אם יתווספו תופעות לוואי לפני require.

הקלות

הוסף מגן כניסה חוזר לכניסה חוזרת של אותה פונקציה והשתמש גם בדפוס אינטראקציות בדיקות-השפעות לכניסה חוזרת מורכבת בין פונקציות.

לא לאפשר העברות בסכום אפס

תיאור

השמיים ZenPoolFunds.sol החוזה מאפשר העברות בסכום אפס בין משתמשים. אמנם כשלעצמו לא מדובר בפגיעות אבטחה, אך זוהי דגימת קוד שיכולה להרחיב את וקטור ההתקפה של תוקף זדוני.

הפגיעות קיימת בגלל transfer ב-ZenPool פולט אירועים, על ידי ביצוע כמות אפס transfer תוקף יכול לפלוט אירועים מרובים שיכולים בתרחישים מסוימים להפעיל היגיון עסקי מחוץ לשרשרת, אפילו בלי להחזיק שום אסימון במאגר.

הקלות

יש צורך בהבנה מעמיקה יותר של העסק. אם אפשר, הסר את הפונקציונליות הזו.

אם יש מקרי שימוש בהם הגיוני להשתמש בהעברות סכום אפס, יש ליישם שכבה נוספת של צ'קים כדי להגביל את השימוש למשתמשים שהם חלק מקבוצה זו.

רישום לא מספיק עבור פונקציות מועדפות

תיאור

הבאים מיוחסים onlyOwner  הפונקציה אינה פולטת אירועים.

אירועים הם נוהג נפוץ לפונקציות מיוחסות להודיע ​​לציבור שבוצע שינוי. ללא אירועים, קשה יותר לזהות שינויים והמשתמשים מופתעים מהתנהגות החוזה.

הבעלים יכול להתקשר ל chainId על ידי ביצוע setChainId() ושינוי ה chaindId פרמטר מבלי שנפלט שום אירוע בתהליך.

הקלות

הוסף קוד שפולט אירועים.

בדיקת מצב מיותר

תיאור

בעת יצירת אסימון מושל חדש ה _timelockDelay הפרמטר מאומת פעמיים. פעם אחת בודק מתי הפונקציה מתחילה ולאחר מכן כחלק מאחר require, זה מיותר ויש להסירו.

קוד כפול מצריך יותר גז להפעלתו ויכול להוביל לבאגים עתידיים כאשר ההיגיון העסקי משתנה.

הקלות

הסר את המיותר השני require.

חוסר עקביות בשמות

תיאור

ישנן מופעים מרובים של סגנונות שונים של אותיות רישיות (מקרה גמל תחתון, מארז נחש וכו').

הקלות

סקור את הקוד עבור עיצוב הקוד.

הוסף מדריך לעיצוב קוד ואכף אותו באמצעות משימת CI.