Submitted by Lambda
HolographERC721.sol#L272
According to EIP-721, we have for approve:
An operator in the context of EIP-721 is someone who was approved via setApprovalForAll:
Besides operators, there are also approved addresses for a token (for which approve is used). However, approved addresses can only transfer the token, see for instance the safeTransferFrom description:
HolographERC721 does not distinguish between authorized operators and approved addresses when it comes to the approve function. Because _isApproved(msg.sender, tokenId) is used there, an approved address can approve another address, which is a violation of the EIP (only authorized operators should be able to do so).
Bob calls approve to approve Alice on token ID 42 (that is owned by Bob). One week later, Bob sees that a malicious address was approved for his token ID 42 (e.g., because Alice got phished) and stole his token. Bob wonders how this is possible, because Alice should not have the permission to approve other addresses. However, becaue HolographERC721 did not follow EIP-721, it was possible.
Follow the EIP, i.e. do not allow approved addresses to approve other addresses.
alexanderattar (Holograph) commented:
gzeon (judge) commented:
Lambda (warden) commented:
gzeon (judge) commented:
ACC01ADE (Holograph) linked a PR:
For this contest, 113 reports were submitted by wardens detailing low risk and non-critical issues. The report highlighted below by Rolezn received the top score from the judge.
The following wardens also submitted reports: 0xNazgul, 0xSmartContract, adriro, Deivitto, lukris02, oyc_109, RaymondFam, rotcivegaf, __141345__, peiw, Picodes, Rahoz, RaoulSchaffranek, ret2basic, ryshaw, sakman, seyni, Shinchan, sikorico, 0xZaharina, Tagir2003, teawaterwire, tnevler, w0Lfrum, Yiko, 8olidity, ajtra, Aymen0909, aysha, B2, bin2chen, Bnke0x0, bobirichman, brgltd, catchup, cccz, cdahlheimer, ch0bu, cryptostellar5, csanuragjain, delfin454000, Diana, djxploit, erictee, fatherOfBlocks, Jeiwan, Josiah, KoKo, leosathya, m_Rassska, martin, mcwildy, mics, nicobevi, peanuts, pedr02b2, rbserver, RedOneN, ReyAdmirado, rvierdiiev, sakshamguruji, saneryee, securerodd, svskaushik, Trust, Waze, 0x1f8b, 0x52, 0x5rings, 0xhunter, 0xzh, a12jmx, Amithuddar, arcoun, ballx, bulej93, catwhiskeys, caventa, chaduke, Chom, chrisdior4, cloudjunky, cryptphi, cylzxje, d3e4, Diraco, Dravee, durianSausage, francoHacker, Franfran, gianganhnguyen, gogo, hansfriese, i_got_hacked, ignacio, imare, JC, JrNet, Jujic, karanctf, KingNFT, kv, Lambda, louhk, lyncurion, malinariy, Margaret, Migue, minhtrng, Ocean_Sky, PaludoX0, and pashov.
Note: See wardens original submission for full details and PoCs on each item below.
