Platform: Code4rena
Start Date: 13/11/2023
Pot Size: $24,500 USDC
Total HM: 3
Participants: 120
Period: 4 days
Judge: 0xTheC0der
Id: 306
League: ETH
Rank: 84/120
Findings: 1
Award: $4.08
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: chaduke
Also found by: 0xpiken, Bauchibred, Matin, MohammedRizwan, MrPotatoMagic, OMEN, Pheonix, SandNallani, T1MOH, Topmark, ZanyBonzy, adriro, aslanbek, ayden, bareli, bart1e, bin2chen, btk, cheatc0d3, codynhat, critical-or-high, d3e4, erebus, firmanregar, hunter_w3b, jasonxiale, kaveyjoe, ksk2345, lsaudit, max10afternoon, merlinboii, nailkhalimov, osmanozdemir1, peanuts, pep7siup, pontifex, sbaudh6, shenwilly, sl1, tourist, wisdomn_, young, zhaojie
4.0797 USDC - $4.08
In order to create a share a user has to call createNewShare() passing shareName, bondingCurve and URI. Currently there exists a check that does not allow creating shares with the same name.
require(shareIDs[_shareName] == 0, "Share already exists");
If the share with the name passed already exists, the tx will simply revert. This approach creates a way for a malicious user to DoS anyone from creating a new share. An attacker can monitor mempool for transactions to createNewShare() and front-run them passing shareName from the original tx. The attacker will only have to pay gas for the transaction, and he can do it indefinetely.
The impact would be inability to interact with the core function of the contract. Yes, there is a way to restrict the creation of shares, but that would require never turning it off and manually whitelisting users to allow them to create a share. It will prevent the protocol from ever being permissionless.
function createNewShare( string memory _shareName, address _bondingCurve, string memory _metadataURI ) external onlyShareCreator returns (uint256 id) { require(whitelistedBondingCurves[_bondingCurve], "Bonding curve not whitelisted"); require(shareIDs[_shareName] == 0, "Share already exists"); <----- @audit id = ++shareCount; shareIDs[_shareName] = id; shareData[id].bondingCurve = _bondingCurve; shareData[id].creator = msg.sender; shareData[id].metadataURI = _metadataURI; emit ShareCreated(id, _shareName, _bondingCurve, msg.sender); }
Manual review
The possible solution is to get rid of this check, allowing creation of the shares with the same name. It may not be a problem, as every function in the contract is quering a share by its id, not name. Alternatively, introduce a fee to createNewShare function, so the user would be required to pay for the creation, this way it will disincentivize the attacker from DoS'ing this function.
DoS
#0 - c4-pre-sort
2023-11-18T16:29:31Z
minhquanym marked the issue as duplicate of #124
#1 - c4-judge
2023-11-29T00:41:47Z
MarioPoneder changed the severity to QA (Quality Assurance)
#2 - c4-judge
2023-11-29T22:43:30Z
MarioPoneder marked the issue as grade-b
#3 - kazantseff
2023-12-01T18:49:40Z
@MarioPoneder - Thank you for judging this contest. I would like to add more context to this issue and raise some concerns.
I understand that the incentives for this attack are limited, but the cost of the attack is in fact low for the protocol deployed on Canto blockchain and even more negligible for the protocol deployed on a different blockchain, take Polygon for example. If we calculate the approximate cost of createNewShare transaction on Canto, it will be around 0.05$ per tx, which will yield roughly 5$ per 100 transactions. In comparison if the protocol deployed on Polygon, an attacker would have to spend only around 6$ to DoS a 1000 transactions. This in my opinion could potentially justify the concern that limited incentivies with a low attack cost may still pose a threat to the availability of the system. The impact is not just wasted gas on the side of the user, but the damage to the availability of the protocol. As i’ve stated in my submission, it can prevent protocol from being permisionless either because the attacker will decide who can create shares, or force the protocol to be in restricted state such that only whitelisted users can participate. I believe this is a potential risk and that it is imporant to ensure protocol availability cannot be negatively impacted by any malicious actors.
I would appreciate your perspective on the matter and a reconsideration of the severity of the issue.
Thank you for your time, kind regards.
#4 - MarioPoneder
2023-12-04T12:23:51Z
Thank your for your comment!
I acknowledge the validity of your concerns. In the end it all boils down to where to set the bar for Medium severity findings.
My my perspective:
Medium severity would be unfair and inconsistent if compared with many other submissions and their impacts that have been downgraded to QA.#5 - kazantseff
2023-12-04T12:28:15Z
@MarioPoneder - I understand and grateful for your opinion!