Platform: Code4rena
Start Date: 18/10/2022
Pot Size: $75,000 USDC
Total HM: 27
Participants: 144
Period: 7 days
Judge: gzeon
Total Solo HM: 13
Id: 170
League: ETH
Rank: 59/144
Findings: 1
Award: $55.67
π Selected for report: 0
π Solo Findings: 0
π Selected for report: Rolezn
Also found by: 0x1f8b, 0x52, 0x5rings, 0xNazgul, 0xSmartContract, 0xZaharina, 0xhunter, 0xzh, 8olidity, Amithuddar, Aymen0909, B2, Bnke0x0, Chom, Deivitto, Diana, Diraco, Dravee, Franfran, JC, Jeiwan, Josiah, JrNet, Jujic, KingNFT, KoKo, Lambda, Margaret, Migue, Ocean_Sky, PaludoX0, Picodes, Rahoz, RaoulSchaffranek, RaymondFam, RedOneN, ReyAdmirado, Shinchan, Tagir2003, Trust, Waze, Yiko, __141345__, a12jmx, adriro, ajtra, arcoun, aysha, ballx, bin2chen, bobirichman, brgltd, bulej93, catchup, catwhiskeys, caventa, cccz, cdahlheimer, ch0bu, chaduke, chrisdior4, cloudjunky, cryptostellar5, cryptphi, csanuragjain, cylzxje, d3e4, delfin454000, djxploit, durianSausage, erictee, fatherOfBlocks, francoHacker, gianganhnguyen, gogo, hansfriese, i_got_hacked, ignacio, imare, karanctf, kv, leosathya, louhk, lukris02, lyncurion, m_Rassska, malinariy, martin, mcwildy, mics, minhtrng, nicobevi, oyc_109, pashov, peanuts, pedr02b2, peiw, rbserver, ret2basic, rotcivegaf, rvierdiiev, ryshaw, sakman, sakshamguruji, saneryee, securerodd, seyni, sikorico, svskaushik, teawaterwire, tnevler, w0Lfrum
55.6726 USDC - $55.67
https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/enforcer/HolographERC20.sol#L392-L410 https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/enforcer/HolographERC20.sol#L556-L558
More than type(uint256).max ERC20 tokens can be minted using HolographERC20
because there is no info how many tokens are bridged.
When HolographERC20.bridgeOut
function is called, it uses burning mechanism for bridging instead of locking. That means that totalSupply is decreasing. And there is no information how many tokens were bridged. That means that we can mint more tokens, up to type(uint256).max
, however we already have more tokens on other chains.
When someone of token holders on another chain will try to bridgeIn
tokens back to the chain, the _mint
function will revert, because of overflow.
Suppose that we have HolographERC20
token that already minted type(uint256).max amount of tokens.
Some userA bridge out 1000 tokens.
HolographERC20
have minted more 1000 tokens through the sourceMint
function.
userA wants to bridge in his 1000 tokens, but can't because of overflow.
VsCode
Lock tokens instead of burning, or calculate bridged amount and control minting to not exceed max amount of supply.
#0 - gzeoneth
2022-10-31T13:52:32Z
Technically true but prolly low risk.
#1 - alexanderattar
2022-11-08T21:08:10Z
This is considered out of scope. Custom contracts implement should be responsible for implementing this logic.
#2 - rvierdiyev
2022-11-14T12:07:07Z
I believe that protocol should protect users from such situations. ERC20 token shouldn't have more tokens than it's total supply in circulation. You give ability for the custom contract creators to mint tokens through sourceMint
function, so actually you allow them to break this amount and should restrict that. Custom contract creator can mint more tokens maliciously or mistakenly.
Custom contracts implement should be responsible for implementing this logic
I believe that custom contracts should be responsible for how they mint tokens, but they should not store information about how many tokens are bridged, this is responsibility of HolographERC20 as it's created for bridging tokens.
#3 - alexanderattar
2022-11-14T21:36:22Z
that's a reasonable argument @rvierdiyev, but we still disagree with this opinion. We do not want to limit what developers can do by setting hard limits at the protocol level. Developers should know how many tokens they want to be able to mint through their contracts. I will change the label to sponsor acknowledged
.
#4 - rvierdiyev
2022-11-15T06:25:07Z
@alexanderattar as a suggestion, you can make custom contract implement some method as totalSupply()(used by developers to set token supply amount) and then use that function inside Holographer to get total supply. In such way you don't restrict developers with total supply amount, but still have control over it.
#5 - gzeoneth
2022-11-21T07:28:21Z
As QA report
#6 - alexanderattar
2022-11-23T03:38:40Z
Thanks for the suggestion @rvierdiyev! We will consider that