Function incorrectly identified as protected
- Dominant language
- Python
- Stars
- 6.4k
- Forks
- 1.1k
- PR merge metrics
- No merged PRs in 30d
Description
### Describe the false alarm that Slither raise and how you know it's inaccurate:
Not a detector FP, but a FP from the `Function.is_protected` function.
In Compound's CErc20 contract. The `mint(uint mintAmount)` function is being incorrectly flagged as protected. I think it is because line [1489 of function.py](https://github.com/crytic/slither/blob/master/slither/core/declarations/function.py#L1489) sees `msg.sender` in the arguments being passed into the `mintFresh(address minter, uint mintAmount)` internal function on [line 389 of CToken.sol](https://github.com/webthethird/solidity-diff-fuzz-upgrades/blob/main/src/implementation/compound/master-contracts/CToken.sol#L389). Yet there are no conditional statements in `mintFresh` which depend on the minter, so it should not be considered protected.
Interestingly, the `redeem`, `redeemUnderlying` and `borrow` functions are not flagged as protected, apparently because they all cast `msg.sender` to a payable address, i.e., `borrowFresh(payable(msg.sender), borrowAmount)` vs. `mintFresh(msg.sender, mintAmount)`.
### Frequency
Very Frequently
### Code example to reproduce the issue:
[CErc20.sol](https://github.com/webthethird/solidity-diff-fuzz-upgrades/blob/main/src/implementation/compound/master-contracts/CErc20.sol)
[CToken.sol](https://github.com/webthethird/solidity-diff-fuzz-upgrades/blob/main/src/implementation/compound/master-contracts/CToken.sol)
```solidity
contract CErc20 is CToken, CErc20Interface {
...
function mint(uint mintAmount) override external returns (uint) {
mintInternal(mintAmount);
return NO_ERROR;
}
...
}
abstract contract CToken is CTokenInterface, ExponentialNoError, TokenErrorReporter {
...
function mintInternal(uint mintAmount) internal nonReentrant {
accrueInterest();
// mintFresh emits the actual Mint event if successful and logs on errors, so we don't need to
mintFresh(msg.sender, mintAmount);
}
function mintFresh(address minter, uint mintAmount) internal {
/* Fail if mint not allowed */
uint allowed = comptroller.mintAllowed(address(this), minter, mintAmount);
if (allowed != 0) {
revert MintComptrollerRejection(allowed);
}
/* Verify market's block number equals current block number */
if (accrualBlockNumber != getBlockNumber()) {
revert MintFreshnessCheck();
}
Exp memory exchangeRate = Exp({mantissa: exchangeRateStoredInternal()});
/////////////////////////
// EFFECTS & INTERACTIONS
// (No safe failures beyond this point)
/*
* We call `doTransferIn` for the minter and the mintAmount.
* Note: The cToken must handle variations between ERC-20 and ETH underlying.
* `doTransferIn` reverts if anything goes wrong, since we can't be sure if
* side-effects occurred. The function returns the amount actually transferred,
* in case of a fee. On success, the cToken holds an additional `actualMintAmount`
* of cash.
*/
uint actualMintAmount = doTransferIn(minter, mintAmount);
/*
* We get the current exchange rate and calculate the number of cTokens to be minted:
* mintTokens = actualMintAmount / exchangeRate
*/
uint mintTokens = div_(actualMintAmount, exchangeRate);
/*
* We calculate the new total supply of cTokens and minter token balance, checking for overflow:
* totalSupplyNew = totalSupply + mintTokens
* accountTokensNew = accountTokens[minter] + mintTokens
* And write them into storage
*/
totalSupply = totalSupply + mintTokens;
accountTokens[minter] = accountTokens[minter] + mintTokens;
/* We emit a Mint event, and a Transfer event */
emit Mint(minter, actualMintAmount, mintTokens);
emit Transfer(address(this), minter, mintTokens);
/* We call the defense hook */
// unused function
// comptroller.mintVerify(address(this), minter, actualMintAmount, mintTokens);
}
### Version:
0.9.2
### Relevant log output:
_No response_
Contributor guide
Assessment
This issue has not been assessed yet.