code-423n4 / 2022-03-joyn-findings

4 stars 1 forks source link

Upgraded Q -> H from 110 [1655009286368] #154

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Judge has assessed an item in Issue #110 as High risk. The relevant finding follows:

platformFee should be upper bounded to avoid DoS and excessive fees

platformFee can take a value of 10000 (100%) which could be seen as a trust issue:

File: RoyaltyVault.sol 67: function setPlatformFee(uint256 _platformFee) external override onlyOwner { 68: platformFee = _platformFee; //@audit low should be upperbounded to 10000 or L41 will get DOSed by an underflow. A reasonable upperbound should be declared for trust 69: emit NewRoyaltyVaultPlatformFee(_platformFee); 70: } Also, although unlikely and remediable by calling again setPlatformFee with another value, sendToSplitter can get DOSed by the admin by setting platformFee to more than 10000:

File: RoyaltyVault.sol 40: uint256 platformShare = (balanceOfVault * platformFee) / 10000; 41: uint256 splitterShare = balanceOfVault - platformShare; //@audit DOSed by the admin if platformFee > 10000, which is possible