Lines of code
<https://github.com/code-423n4/2023-11-canto/blob/335930cd53cf9a137504a57f1215be52c6d67cb3/1155tech-contracts/src/Market.sol#L229>
<https://github.com/code-423n4/2023-11-canto/blob/335930cd53cf9a137504a57f1215be52c6d67cb3/1155tech-contracts/src/Market.sol#L153>
Fee calculations depends on shareData[_id].tokenCount, which is updated AFTER doing the transfer of token. That means, if the token is an ERC777 compatible token, users can reenter the function paying, for example, less fees on a buy operation.
NOTE --> Iām gonna work on top of the buy operation, the others are kind of the same
When users call Market, function buy, the fee they are gonna pay is retrieved by calling getBuyPrice
(uint256 price, uint256 fee) = getBuyPrice(_id, _amount); // Reverts for non-existing ID
whose implementation is
function getBuyPrice(uint256 _id, uint256 _amount) public view returns (uint256 price, uint256 fee) {
// If id does not exist, this will return address(0), causing a revert in the next line
address bondingCurve = shareData[_id].bondingCurve;
(price, fee) = IBondingCurve(bondingCurve).getPriceAndFee(shareData[_id].tokenCount + 1, _amount);
}
which depends on shareData[_id].tokenCount, used as the argument to log2 in LinearBondingCurve, function getFee.
After that, Market pulls price + fee tokens from the user and AFTER that, it updates shareData[_id].tokenCount. That means, if token is an ERC777 token with the given hooks on transfers, it can reenter the function and buy far more tokens just by paying the same amount of fees, which leads to a theft of fees from the Market
SafeERC20.safeTransferFrom(token, msg.sender, address(this), price + fee);
// The reward calculation has to use the old rewards value (pre fee-split) to not include the fees of this buy
// The rewardsLastClaimedValue then needs to be updated with the new value such that the user cannot claim fees of this buy
uint256 rewardsSinceLastClaim = _getRewardsSinceLastClaim(_id);
// Split the fee among holder, creator and platform
_splitFees(_id, fee, shareData[_id].tokensInCirculation);
rewardsLastClaimedValue[_id][msg.sender] = shareData[_id].shareHolderRewardsPerTokenScaled;
shareData[_id].tokenCount += _amount;
Just make nonReentrant those functions.
Reentrancy
The text was updated successfully, but these errors were encountered:
š 1 ustas-eth reacted with confused emoji
All reactions