Platform: Code4rena
Start Date: 07/07/2022
Pot Size: $75,000 USDC
Total HM: 32
Participants: 141
Period: 7 days
Judge: HardlyDifficult
Total Solo HM: 4
Id: 144
League: ETH
Rank: 92/141
Findings: 2
Award: $76.58
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: 0xA5DF
Also found by: 0x52, 0xDjango, 0xsanson, Lambda, PwnedNoMore, Ruhum, Treasure-Seeker, async, berndartmueller, cccz, hubble, kenzo, scaraven, shenwilly, sseefried, xiaoming90
14.6423 USDC - $14.64
https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/modules/Migration.sol#L210 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/modules/Buyout.sol#L57
As part of the migration of a vault, the commit function is called to kickoff the buyout process for the migration. Its possible that any EOA address can initiate a start function on the Buyout module with a very low amount of msg.value and prevent the above migration-commit to revert, by front-running it.
Impact the migration process. Denial of service attack can be done repeatedly, so that migration is postponed indefinitely.
Contract : Migration.sol, function commit() Contract : Buyout.sol, function start()
Alternate design with either permissioned call to start function or queueing the requests for the start function call.
#0 - mehtaculous
2022-07-18T16:42:27Z
Duplicate of #87
#1 - HardlyDifficult
2022-08-02T21:54:42Z
🌟 Selected for report: xiaoming90
Also found by: 0x1f8b, 0x29A, 0x52, 0xA5DF, 0xDjango, 0xNazgul, 0xNineDec, 0xf15ers, 0xsanson, 0xsolstars, 242, 8olidity, Amithuddar, Aymen0909, Bnke0x0, BowTiedWardens, David_, Deivitto, ElKu, Funen, Hawkeye, IllIllI, JC, Kaiziron, Keen_Sheen, Kthere, Kulk0, Kumpa, Lambda, MEP, ReyAdmirado, Rohan16, Ruhum, Sm4rty, TomJ, Tomio, Treasure-Seeker, TrungOre, Tutturu, Viksaa39, Waze, _Adam, __141345__, ak1, apostle0x01, asutorufos, async, ayeslick, aysha, bbrho, benbaessler, berndartmueller, c3phas, cccz, chatch, cloudjunky, codexploder, cryptphi, delfin454000, dipp, durianSausage, dy, exd0tpy, fatherOfBlocks, hake, hansfriese, horsefacts, hubble, joestakey, jonatascm, kebabsec, kenzo, kyteg, mektigboy, neumo, oyc_109, pashov, pedr02b2, peritoflores, rajatbeladiya, rbserver, robee, rokinot, s3cunda, sach1r0, sahar, sashik_eth, scaraven, shenwilly, simon135, sorrynotsorry, sseefried, svskaushik, unforgiven, z3s, zzzitron
61.9379 USDC - $61.94
https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/modules/protoforms/BaseVault.sol#L65 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/references/TransferReference.sol#L22
The return values of the tranfer/transferFrom call is not checked. There is a possibility that with some ERC20 implementation, failures are not handled properly.
Not checking the return values, will assume that transfer was successfull, and may impact accounting further in the contract.
Contract : BaseVault.sol Function : batchDepositERC20(), Line#65 2. Contract : TransferReference.sol Function : ERC20Transfer(), Line#22
Use the safe transfer versions of these commands from Openzeppelin.
#0 - stevennevins
2022-07-20T21:07:45Z
Duplicate of #312
#1 - HardlyDifficult
2022-07-26T17:42:58Z
Agree with the sponsor that this is a non-critical best practice. Transfers may fail but no events are emitted, balance doesn't change, and no other negative consequences were identified.
Lowering risk and making this a QA report for the warden.