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: 160/175
Findings: 1
Award: $0.11
🌟 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
When the PayableCall[] is not the msg.sender, payableCall() will be made on behalf of the msg.sender because no access control. As a result each of the functions called by payableCall() will be made on behalf of PayableCall[] target address. This can indeed lead to unexpected and potentially fund loss behaviour.
struct PayableCall { address target; bytes callData; uint256 value; } function payableCall(PayableCall[] calldata calls) public payable returns (bytes[] memory returnData) { uint256 valAccumulator; uint256 length = calls.length; returnData = new bytes[](length); PayableCall calldata _call; for (uint256 i = 0; i < length;) { _call = calls[i]; uint256 val = _call.value; unchecked { valAccumulator += val; } bool success; if (isContract(_call.target)) (success, returnData[i]) = _call.target.call{value: val}(_call.callData); if (!success) revert CallFailed(); unchecked { ++i; } } if (msg.value != valAccumulator) revert CallFailed(); }
Manual code review
Use Modifier requiresApprovedCaller
Access Control
#0 - c4-pre-sort
2023-10-08T14:05:49Z
0xA5DF marked the issue as duplicate of #888
#1 - c4-pre-sort
2023-10-08T14:38:08Z
0xA5DF marked the issue as sufficient quality report
#2 - c4-judge
2023-10-26T11:29:34Z
alcueca marked the issue as satisfactory
#3 - c4-judge
2023-10-26T11:32:08Z
alcueca changed the severity to 3 (High Risk)
🌟 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
Allowing anyone to trigger the payableCall() with an empty PayableCall[] array within a loop can indeed lead to a Denial of Service (DoS) attack. In this case an attacker repeatedly calls payableCall with an empty array within a loop, they can consume a significant amount of gas with each call, effectively causing a spike in transaction costs for the sender. If the attacker executes this attack repeatedly, it can lead to a situation where legitimate users are unable to afford the high gas costs required to interact with the contract, effectively denying service to legitimate users.
function payableCall(PayableCall[] calldata calls) public payable returns (bytes[] memory returnData) { uint256 valAccumulator; uint256 length = calls.length; returnData = new bytes[](length); PayableCall calldata _call; for (uint256 i = 0; i < length;) { _call = calls[i]; uint256 val = _call.value; unchecked { valAccumulator += val; } bool success; if (isContract(_call.target)) (success, returnData[i]) = _call.target.call{value: val}(_call.callData); if (!success) revert CallFailed(); unchecked { ++i; } } if (msg.value != valAccumulator) revert CallFailed(); }
Manual code review
Use Modifier requiresApprovedCaller or check PayableCall[] is not empty
DoS
#0 - c4-pre-sort
2023-10-08T14:05:35Z
0xA5DF marked the issue as duplicate of #888
#1 - c4-pre-sort
2023-10-08T14:37:59Z
0xA5DF marked the issue as sufficient quality report
#2 - c4-judge
2023-10-26T11:29:31Z
alcueca marked the issue as satisfactory
#3 - c4-judge
2023-10-26T11:32:08Z
alcueca changed the severity to 3 (High Risk)