Platform: Code4rena
Start Date: 24/03/2023
Pot Size: $49,200 USDC
Total HM: 20
Participants: 246
Period: 6 days
Judge: Picodes
Total Solo HM: 1
Id: 226
League: ETH
Rank: 188/246
Findings: 1
Award: $13.13
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: brgltd
Also found by: 0x3b, 0xAgro, 0xGusMcCrae, 0xNorman, 0xRajkumar, 0xSmartContract, 0xTraub, 0xWagmi, 0xWaitress, 0xffchain, 0xhacksmithh, 0xkazim, 0xnev, 3dgeville, ArbitraryExecution, Aymen0909, BRONZEDISC, Bason, Bloqarl, BlueAlder, Brenzee, CodeFoxInc, CodingNameKiki, Cryptor, DadeKuma, DevABDee, Diana, Dug, Englave, Gde, Haipls, HollaDieWaldfee, Ignite, Infect3d, Jerry0x, Josiah, Kaysoft, Koko1912, KrisApostolov, Lavishq, LeoGold, Madalad, PNS, Rappie, RaymondFam, RedTiger, Rickard, Rolezn, Sathish9098, SunSec, T1MOH, UdarTeam, Udsen, Viktor_Cortess, Wander, adriro, ak1, alejandrocovrr, alexzoid, arialblack14, ayden, bin2chen, brevis, btk, c3phas, carlitox477, catellatech, ch0bu, chaduke, ck, climber2002, codeslide, descharre, dingo2077, ernestognw, fatherOfBlocks, favelanky, georgits, helios, hl_, inmarelibero, juancito, ks__xxxxx, lopotras, lukris02, m_Rassska, mahdirostami, maxper, nadin, navinavu, nemveer, p_crypt0, peanuts, pipoca, pixpi, qpzm, rbserver, reassor, roelio, rotcivegaf, scokaf, siddhpurakaran, slvDev, smaul, tnevler, tsvetanovv, turvy_fuzz, vagrant, wen, yac, zzzitron
13.1298 USDC - $13.13
1.Function Calls in Loop Could Lead to Denial of Service ->Function calls made in unbounded loop are error-prone with potential resource exhaustion as it can trap the contract due to the gas limitations or failed transactions. ->Here are some of the instances entailed: https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L71
->Consider bounding the loop where possible to avoid unnecessary gas wastage and denial of service.
2.Add a Timelock to Critical Parameter Change ->It is a good practice to give time for users to react and adjust to critical changes with a mandatory time window between them. The first step merely broadcasts to users that a particular change is coming, and the second step commits that change after a suitable waiting period. This allows users that do not accept the change to withdraw within the grace period. A timelock provides more guarantees and reduces the level of trust required, thus decreasing risk for users. It also indicates that the project is legitimate (less risk of a malicious Owner making any malicious or ulterior intention). Specifically, privileged roles could use front running to make malicious changes just ahead of incoming transactions, or purely accidental negative effects could occur due to the unfortunate timing of changes. ->Consider extending the timelock feature beyond contract ownership management to business critical functions. ->Here are some of the instances entailed: https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L165
3.Check if the new value is already the same as the old one. ->Many functions are used for updating values of certain state variables. ->In those functions first check if the new value provided is not the same as the old value and then only change it, if new value is different than old one.
->Here are some of the instances entailed: https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L165
4.Emit both old and new value when critical state variable value gets updated ->Currently code just emits new values when critical state variables like minAmount, maxAmount and many others get updated. ->So emit both new and old value when critical state variable gets updated
->Here are some of the instances entailed: https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L165
4.Validate User Inputs correctly ->A.Invalid out of bound index values as function parameter --->In some functions parameters user can pass incorrect value as example derivativeIndex. --->This may execute code for out of bound indexes passed by the user and ultimately the call will be reverted. --->So add require check for function parameters like derivativeIndex
--->Here are some of the instances entailed: https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L165
->B.Unstack function is not validating safEthAmount passed by user in function parameter. --->So the user can request more than his holding amount and the unstake function is actually running withdraw from all derivative contracts. After that on Line number 120, burn will be reverted because of less balance than users holding. --->So in short we should have to validate the amount entered by the user at the start of the unstake function. --->Add require statement in line number 110 require(_safEthAmount <= balanceOf(msg.sender),"provide valid amount");
#0 - c4-sponsor
2023-04-07T22:24:19Z
toshiSat marked the issue as sponsor acknowledged
#1 - c4-judge
2023-04-24T17:28:13Z
Picodes marked the issue as grade-b