FundingPool Static Analysis Findings

FPL-01S: Inexistent Sanitization of Input Addresses

Type	Severity	Location
Input Sanitization		FundingPool.sol:L27-L30

Description:

The linked function(s) accept address arguments yet do not properly sanitize them.

Impact:

The presence of zero-value addresses, especially in constructor implementations, can cause the contract to be permanently inoperable. These checks are advised as zero-value inputs are a common side-effect of off-chain software related bugs.

Example:

Copy
27constructor(address _loanProposalFactory, address _depositToken) {
28    loanProposalFactory = _loanProposalFactory;
29    depositToken = _depositToken;
30}

Recommendation:

We advise some basic sanitization to be put in place by ensuring that each address specified is non-zero.

Alleviation (c740f7c6b5ebd365618fd2d7ea77370599e1ca11):

Both input variables of the FundingPoolImpl::initialize which imitates the previously-specified FundingPool::constructor are properly sanitized against the zero address in the latest implementation, ensuring that the contract cannot be accidentally misconfigured during its deployment.

FPL-02S: Insecure Deposit Methodology

Type	Severity	Location
Language Specific		FundingPool.sol:L36-L42

Description:

The FundingPool::deposit function will measure the actual balance deposited dynamically using ERC20::balanceOf measurements which is insecure when dealing with tokens that permit re-entrancies to occur.

Impact:

It is presently possible to arbitrarily inflate one's EIP-20 balance by redepositing during a re-entrancy, causing the consequent deposit to be counted twice; once during the inner-level FundingPool::deposit transaction and once in the dynamic balance measurement of the top-level FundingPool::deposit transaction. This can be repeated as many times as desired to exacerbate the inflation of one's deposit.

The exhibit has been marked as "minor" due to the assumption that the Myso Finance system is not expected to be compatible with re-entrant tokens, however, if such support is expected we will upgrade the severity of this exhibit to "major".

Example:

Copy
32function deposit(uint256 amount, uint256 transferFee) external {
33    if (amount == 0) {
34        revert Errors.InvalidSendAmount();
35    }
36    uint256 preBal = IERC20Metadata(depositToken).balanceOf(address(this));
37    IERC20Metadata(depositToken).safeTransferFrom(
38        msg.sender,
39        address(this),
40        amount
41    );
42    uint256 postBal = IERC20Metadata(depositToken).balanceOf(address(this));
43    if ((postBal - preBal) != amount - transferFee) {
44        revert Errors.InvalidSendAmount();
45    }
46    balanceOf[msg.sender] += postBal - preBal;
47}

Recommendation:

We advise the FundingPool::deposit function to be marked as non-reentrant, preventing this type of vulnerability from manifesting.

Alleviation (c740f7c6b5ebd365618fd2d7ea77370599e1ca11):

The FundingPool::deposit function has been properly marked as nonReentrant via the ReentrancyGuard dependency of OpenZeppelin, alleviating this exhibit.