Platform: Code4rena
Start Date: 22/09/2023
Pot Size: $100,000 USDC
Total HM: 15
Participants: 175
Period: 14 days
Judge: alcueca
Total Solo HM: 4
Id: 287
League: ETH
Rank: 84/175
Findings: 2
Award: $25.79
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: 0xTheC0der
Also found by: 0x180db, 0xDING99YA, 0xRstStn, 0xTiwa, 0xWaitress, 0xblackskull, 0xfuje, 3docSec, Aamir, Black_Box_DD, HChang26, Hama, Inspecktor, John_Femi, Jorgect, Kek, KingNFT, Kow, Limbooo, MIQUINHO, MrPotatoMagic, NoTechBG, Noro, Pessimistic, QiuhaoLi, SovaSlava, SpicyMeatball, T1MOH, TangYuanShen, Vagner, Viktor_Cortess, Yanchuan, _eperezok, alexweb3, alexxander, ast3ros, ayden, bin2chen, blutorque, btk, ciphermarco, ether_sky, gumgumzum, gztttt, hals, imare, its_basu, joaovwfreire, josephdara, klau5, kodyvim, ladboy233, marqymarq10, mert_eren, minhtrng, n1punp, nobody2018, oada, orion, peakbolt, peritoflores, perseverancesuccess, pfapostol, rvierdiiev, stuxy, tank, unsafesol, ustas, windhustler, zambody, zzzitron
0.1127 USDC - $0.11
https://github.com/code-423n4/2023-09-maia/blob/main/src/VirtualAccount.sol#L85
All the non-view and non-pure functions in the VirtualAccount contract have the requiresApprovedCaller modifier, except the payableCall function. This would allow anyone to make arbitrary calls and steal the user's funds.
Manual Review
Add the requiresApprovedCaller modifier to the payableCall function as well.
Access Control
#0 - c4-pre-sort
2023-10-08T14:03:42Z
0xA5DF marked the issue as duplicate of #888
#1 - c4-pre-sort
2023-10-08T14:37:08Z
0xA5DF marked the issue as sufficient quality report
#2 - c4-judge
2023-10-26T11:29:08Z
alcueca marked the issue as satisfactory
🌟 Selected for report: MrPotatoMagic
Also found by: 0xAadi, 0xDING99YA, 0xDemon, 0xRstStn, 0xSmartContract, 0xStriker, 0xWaitress, 0xbrett8571, 0xfuje, 0xsagetony, 0xsurena, 33BYTEZZZ, 3docSec, 7ashraf, ABA, ABAIKUNANBAEV, Aamir, Audinarey, Bauchibred, Black_Box_DD, Daniel526, DanielArmstrong, DanielTan_MetaTrust, Dinesh11G, Eurovickk, Franklin, Inspecktor, John, Jorgect, Joshuajee, K42, Kek, Koolex, LokiThe5th, MIQUINHO, Myd, NoTechBG, QiuhaoLi, SanketKogekar, Sathish9098, Sentry, Soul22, SovaSlava, Stormreckson, Tendency, Topmark, Udsen, V1235816, Viktor_Cortess, Viraz, Yanchuan, ZdravkoHr, Zims, albahaca, albertwh1te, alexweb3, alexxander, ast3ros, audityourcontracts, bareli, bin2chen, bronze_pickaxe, c0pp3rscr3w3r, cartlex_, castle_chain, chaduke, debo, ether_sky, gumgumzum, imare, its_basu, jaraxxus, jasonxiale, josephdara, kodyvim, ladboy233, lanrebayode77, lsaudit, mert_eren, minhtrng, n1punp, nadin, niroh, nmirchev8, orion, peakbolt, perseverancesuccess, pfapostol, ptsanev, rvierdiiev, saneryee, shaflow2, te_aut, terrancrypt, twcctop, unsafesol, ustas, versiyonbir, windhustler, yongskiws, zhaojie, ziyou-
25.6785 USDC - $25.68
https://github.com/code-423n4/2023-09-maia/blob/main/src/CoreBranchRouter.sol#L62-L79
The EIP20 standard, in regard to the decimals() function states "OPTIONAL - This method can be used to improve usability, but interfaces and other contracts MUST NOT expect these values to be present."
If an underlying token does not implement the decimals method, the line "uint8 decimals = ERC20(_underlyingAddress).decimals();" will cause the transaction to revert.
Manual Review
Add a try/catch block, and in catch block set the decimals to a default value, maybe 18.
DoS
#0 - c4-pre-sort
2023-10-09T10:55:14Z
0xA5DF marked the issue as duplicate of #649
#1 - c4-pre-sort
2023-10-09T10:56:04Z
0xA5DF marked the issue as sufficient quality report
#2 - c4-judge
2023-10-23T05:37:07Z
alcueca changed the severity to QA (Quality Assurance)
#3 - c4-judge
2023-10-23T05:38:09Z
alcueca marked the issue as grade-a