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: 75/100
Findings: 1
Award: $78.88
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 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.881 USDC - $78.88
The NameWrapper contract allows the direct wrapping of ERC721 .eth name tokens by leveraging the ERC721 standard's onERC721Received hook. This is achieved by decoding certain parameters from the data parameter which are expected to be packed along with the safeTransferFrom call. Most critically the new owner of the wrapped name is determined based on the passed data.
As part of a phishing attack an attacker could simply change the owner in the data parameter. Even a careful user who checks the address of the contract they're interacting with, function selector, recipient address and token being sent could still successfully be tricked due to a small change in the unstructured data. Common web3 interfaces such as Metamask and hardware wallets such as Trezor give users the ability to more easily parse and verify function parameters but not arbitrary byte parameters.
Arguably assuming a user has been tricked into going to a malicious UI is already a strong a assumption, this has nevertheless been submitted as a medium severity issue as the current contract design strongly weakens a verification tool common users should be able to utilize to verify the integrity transactions.
Exploit scenario:
data being set to an attacker controlled addressNameWrapper contractAdd more verification of the decoded data. This could be achieved by requiring the addition of an extra signature to the packed data. Specifically I'd recommend packing the added parameters into a structure according to EIP-712, have the user sign the struct and then pack the signature along with the parameters into the data parameter. Since many wallets already natively support EIP712, this will ensure that users will practically have the chance to properly validate all transaction parameters before submitting.
#0 - jefflau
2022-07-27T07:35:16Z
Recommend reducing severity to 0.
If a user is willing to transfer their name to some smart contract address with opaque data, not including an owner address won't make any difference. If you can convince them to do that you can almost certainly convince them to just send it to you.
#1 - dmvt
2022-08-04T21:24:34Z
Yeah I agree with the sponsor here. This is a decent recommendation, but it doesn't really solve the "user blindly trusting ENS" issue. Downgrading to QA.