Platform: Code4rena
Start Date: 08/09/2023
Pot Size: $70,000 USDC
Total HM: 8
Participants: 84
Period: 6 days
Judge: gzeon
Total Solo HM: 2
Id: 285
League: ETH
Rank: 74/84
Findings: 1
Award: $12.79
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: castle_chain
Also found by: 0xAadi, 0xHelium, 0xLook, 0xblackskull, 0xfuje, 0xmystery, 0xnev, 0xpiken, 7ashraf, BARW, Bauchibred, Bughunter101, Ch_301, JP_Courses, Kaysoft, Krace, MohammedRizwan, SanketKogekar, Sathish9098, alexzoid, ast3ros, btk, catellatech, degensec, fatherOfBlocks, grearlake, imtybik, jkoppel, jolah1, klau5, lsaudit, m_Rassska, merlin, mrudenko, nobody2018, rokinot, rvierdiiev, sandy
12.7917 USDC - $12.79
The @dev comment of transferOut function suggests transferOut can only be initiated by the destination address or an authorized admin but auth modifier prevents that. It only allows selected wards but making every destination address a ward is not feasible as ward in auth modifier is a trusted role and has access to all the major functionality in the protocol.
It bricks a major functionality. The tokens can only be transferred out by authorized ward. Thus, the whole design of transferOut is faulty.
Not required
Manual Analysis
This can be mitigated by only allowing authorized ward to call transferOut function.
Other
#0 - c4-pre-sort
2023-09-16T00:20:27Z
raymondfam marked the issue as sufficient quality report
#1 - c4-pre-sort
2023-09-16T00:20:53Z
raymondfam marked the issue as duplicate of #217
#2 - c4-judge
2023-09-26T13:23:37Z
gzeon-c4 marked the issue as not a duplicate
#3 - c4-judge
2023-09-26T13:23:52Z
gzeon-c4 marked the issue as primary issue
#4 - c4-judge
2023-09-26T13:38:04Z
gzeon-c4 marked the issue as unsatisfactory: Invalid
#5 - c4-judge
2023-09-26T13:39:28Z
gzeon-c4 removed the grade
#6 - gzeon-c4
2023-09-26T13:41:22Z
This is called from InvestmentManager which is a ward of UserEscrow, however it still seems to require auth on the IM to call anything to lead to transferOut. Looks like an ambiguous comment transferOut can only be initiated by the destination address or an authorized admin.
#7 - hieronx
2023-09-26T13:59:03Z
Yeah the implementation is correct, but the comment is wrong/misleading. It should be transferOut can only be initiated by an authorized admin, to a destination set on transferIn.
#8 - c4-judge
2023-09-26T14:25:06Z
gzeon-c4 changed the severity to QA (Quality Assurance)