Platform: Code4rena
Start Date: 12/07/2022
Pot Size: $75,000 USDC
Total HM: 16
Participants: 100
Period: 7 days
Judge: LSDan
Total Solo HM: 7
Id: 145
League: ETH
Rank: 71/100
Findings: 2
Award: $84.32
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: rajatbeladiya
Also found by: 0x29A, 0xNineDec, Amithuddar, Aussie_Battlers, Ch_301, Dravee, GimelSec, IllIllI, Jujic, Limbooo, RedOneN, Ruhum, TomJ, _Adam, __141345__, alan724, asutorufos, berndartmueller, c3phas, cccz, cryptphi, durianSausage, fatherOfBlocks, hake, hyh, pashov, scaraven, zzzitron
5.45 USDC - $5.45
https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/ethregistrar/ETHRegistrarController.sol#L183 https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/ethregistrar/ETHRegistrarController.sol#L204 https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/ethregistrar/ETHRegistrarController.sol#L211 https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/ethregistrar/BulkRenewal.sol#L67
Impact: The use of the deprecated transfer() function for an address will inevitably make the transaction fail when:
Additionally, using higher than 2300 gas might be mandatory for some multisig wallets.
Recommended Mitigation Steps: Use **** call() instead of transfer()
#0 - jefflau
2022-07-22T09:51:25Z
Duplicate of #133
🌟 Selected for report: IllIllI
Also found by: 0x1f8b, 0x29A, 0xDjango, 0xNazgul, 0xNineDec, 0xf15ers, 8olidity, Aussie_Battlers, Bnke0x0, Ch_301, Critical, Deivitto, Dravee, ElKu, Funen, GimelSec, JC, JohnSmith, Lambda, MiloTruck, PwnedNoMore, ReyAdmirado, Rohan16, Rolezn, Ruhum, RustyRabbit, Sm4rty, TomJ, Waze, _Adam, __141345__, alan724, asutorufos, benbaessler, berndartmueller, bin2chen, brgltd, bulej93, c3phas, cRat1st0s, cryptonue, cryptphi, csanuragjain, delfin454000, dxdv, exd0tpy, fatherOfBlocks, gogo, hake, hyh, joestakey, kyteg, lcfr_eth, minhtrng, p_crypt0, pashov, pedr02b2, philogy, rajatbeladiya, rbserver, rishabh, robee, rokinot, sach1r0, sashik_eth, seyni, simon135, svskaushik, zuhaibmohd, zzzitron
78.8735 USDC - $78.87
https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/dnssec-oracle/Owned.sol#L18 https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/root/Ownable.sol#L18
Proof of concept: The current ownership transfer pattern is calling transferOwnership() or setOwner() function with an address type argument, which is immediately set to the new owner of the contract. If the new address of the new owner is actually not a valid account then the ownership just got transferred to an uncontrolled account, breaking all functions that can be called only by owner.
Impact: this can impact availability of the protocol. Especially the problem in Ownable.sol, it can result in no way to withdraw funds from ETHRegistrarController
Recommended Mitigation Steps: Implement a two-step process where the owner nominates an account and the nominated account needs to call an acceptOwnership() function for the transfer of ownership to be completed. This ensures the nominated account is a valid & active one.
#0 - makoto
2022-07-27T10:39:51Z
#1 - dmvt
2022-08-04T23:39:55Z
Lack of 2 step ownership confirmation is considered a QA issue by consensus of the C4 judging body. Downgrading.