Wild Credit contest - gpersoon's results

Handle

gpersoon

Vulnerability details

Impact

Both the functions setLiqParamsToken and setLiqParamsDefault have a check to make sure that _liqFeeCaller + _liqFeeSystem <= MAX_LIQ_FEES

However the constructor of Controller sets the same parameters and doesn't have this check. It seems logical to also do the check in the controller otherwise the parameters could be set outside of the wanted range.

Proof of Concept

// https://github.com/code-423n4/2021-07-wildcredit/blob/main/contracts/Controller.sol#L49 constructor( address _interestRateModel, uint _liqFeeSystemDefault, uint _liqFeeCallerDefault) { ... liqFeeSystemDefault = _liqFeeSystemDefault; liqFeeCallerDefault = _liqFeeCallerDefault;

function setLiqParamsToken( address _token, uint _liqFeeSystem, uint _liqFeeCaller ) external onlyOwner { require(_liqFeeCaller + _liqFeeSystem <= MAX_LIQ_FEES, "Controller: fees too high"); ... liqFeeSystemToken[_token] = _liqFeeSystem; liqFeeCallerToken[_token] = _liqFeeCaller;

function setLiqParamsDefault( uint _liqFeeSystem, uint _liqFeeCaller) external onlyOwner { require(_liqFeeCaller + _liqFeeSystem <= MAX_LIQ_FEES, "Controller: fees too high"); liqFeeSystemDefault = _liqFeeSystem; liqFeeCallerDefault = _liqFeeCaller;

Tools Used

Recommended Mitigation Steps

Add something like the following in the constructor of Controller require(liqFeeCallerDefault + liqFeeSystemDefault <= MAX_LIQ_FEES, "Controller: fees too high");

Handle

gpersoon

Vulnerability details

Impact

The parameter minBorrowUSD of the contract Controller isn't initialized. If someone is able to Borrow before the function setMinBorrowUSD is called, he might be able to borrow a very small amount. This might be unwanted.

Proof of Concept

//https://github.com/code-423n4/2021-07-wildcredit/blob/main/contracts/Controller.sol#L27 uint public minBorrowUSD;

function setMinBorrowUSD(uint _value) external onlyOwner { minBorrowUSD = _value; }

//https://github.com/code-423n4/2021-07-wildcredit/blob/main/contracts/LendingPair.sol#L553 function _checkBorrowLimits(address _token, address _account) internal view { ... require(accountBorrowUSD >= controller.minBorrowUSD(), "LendingPair: borrow amount below minimum");

Tools Used

Recommended Mitigation Steps

Initialize minBorrowUSD via the constructor or set a reasonable default in the contract.

Handle

gpersoon

Vulnerability details

Impact

The function withdrawBorrowETH of the contract LendingPair calls _wethWithdrawTo and then calls _checkMinReserve. However _wethWithdrawTo also calls _checkMinReserve (except when _amount but then not much happens anyway.

So the call to _checkMinReserve in withdrawBorrowETH is redundant and uses some extra gas.

Proof of Concept

//https://github.com/code-423n4/2021-07-wildcredit/blob/main/contracts/LendingPair.sol#L106

function withdrawBorrowETH(uint _amount) external { .. _wethWithdrawTo(msg.sender, _amount); _checkMinReserve(address(WETH)); // is also called in _wethWithdrawTo }

function _wethWithdrawTo(address _to, uint _amount) internal override { if (_amount > 0) { TransferHelper._wethWithdrawTo(_to, _amount); _checkMinReserve(address(WETH)); } }

Tools Used

Recommended Mitigation Steps

Consider removing the _checkMinReserve in withdrawBorrowETH Or consider moving the _checkMinReserve to all functions where _wethWithdrawTo is called