Canto contest - cccz's results

Lines of code

https://github.com/Plex-Engineer/lending-market/blob/755424c1f9ab3f9f0408443e6606f94e4f08a990/contracts/WETH.sol#L85-L88

Vulnerability details

Impact

The approve function of the WETH contract allows the user to spend anyone else's weth

    function approve(address owner, address spender) external returns(bool) {
	    _approve(owner, spender, _balanceOf[owner]);
        return true;
    }
    function _approve(
        address owner,
        address spender,
        uint256 amount
    ) internal   {
        require(owner != address(0), "ERC20: approve from the zero address");
        require(spender != address(0), "ERC20: approve to the zero address");

        _allowance[owner][spender] = amount;
        emit Approval(owner, spender, amount);
    }

Proof of Concept

https://github.com/Plex-Engineer/lending-market/blob/755424c1f9ab3f9f0408443e6606f94e4f08a990/contracts/WETH.sol#L85-L88

Tools Used

None

Recommended Mitigation Steps

Modify the approve function as follows

    function approve(address spender) external returns(bool) {
	_approve(msg.sender, spender, _balanceOf[msg.sender]);
        return true;
    }

Lines of code

https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Accountant/AccountantDelegate.sol#L74-L92

Vulnerability details

Impact

When the user borrows note tokens, the AccountantDelegate contract provides note tokens and gets cnote tokens. Later, when the user repays the note tokens, the cnote tokens are destroyed and the note tokens are transferred to the AccountantDelegate contract. However, in the sweepInterest function of the AccountantDelegate contract, all cnote tokens in the contract will be transferred to address 0. This will prevent the user from repaying the note tokens, and the sweepInterest function will not calculate the interest correctly later.

Proof of Concept

https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Accountant/AccountantDelegate.sol#L74-L92 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/CToken.sol#L533

Tools Used

None

Recommended Mitigation Steps

    function sweepInterest() external override returns(uint) {
		
		uint noteBalance = note.balanceOf(address(this));
		uint CNoteBalance = cnote.balanceOf(address(this));

		Exp memory expRate = Exp({mantissa: cnote.exchangeRateStored()}); // obtain exchange Rate from cNote Lending Market as a mantissa (scaled by 1e18)
		uint cNoteConverted = mul_ScalarTruncate(expRate, CNoteBalance); //calculate truncate(cNoteBalance* mantissa{expRate})
		uint noteDifferential = sub_(note.totalSupply(), noteBalance); //cannot underflow, subtraction first to prevent against overflow, subtraction as integers

		require(cNoteConverted >= noteDifferential, "Note Loaned to LendingMarket must increase in value");
		
		uint amtToSweep = sub_(cNoteConverted, noteDifferential);

		note.transfer(treasury, amtToSweep);

-		cnote.transfer(address(0), CNoteBalance);

		return 0;
    }

Lines of code

https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Accountant/AccountantDelegate.sol#L28-L29

Vulnerability details

Impact

In the initialize function of the AccountantDelegate contract, note._mint_to_Accountant is called. _mint_to_Accountant will mint note tokens to the AccountantDelegate contract instead of msg.sender. if note._initialSupply() ! = 0, note.balanceOf(msg.sender) == note._initialSupply() is false, the initialize function fails.

Proof of Concept

https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Accountant/AccountantDelegate.sol#L28-L29 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Note.sol#L13-L19 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/ERC20.sol#L32 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/ERC20.sol#L242

Tools Used

None

Recommended Mitigation Steps

Make sure _initialSupply == 0

Lines of code

https://github.com/Plex-Engineer/manifest/blob/688e9b4e7835854c22ef44b045d6d226b784b4b8/contracts/Proposal-Store.sol#L46-L50

Vulnerability details

Impact

In the ProposalStore contract, AddProposal can be called by anyone, so that anyone can add a new proposal or overwrite an old proposal.

    function AddProposal(uint propId, string memory title, string memory desc, address[] memory targets, 
                        uint[] memory values, string[] memory signatures, bytes[] memory calldatas) public {
        Proposal memory newProp = Proposal(propId, title, desc, targets, values, signatures, calldatas);
        proposals[propId] = newProp;
    }

Proof of Concept

https://github.com/Plex-Engineer/manifest/blob/688e9b4e7835854c22ef44b045d6d226b784b4b8/contracts/Proposal-Store.sol#L46-L50

Tools Used

None

Recommended Mitigation Steps

Add the OnlyUniGov modifier to the AddProposal function

Lines of code

https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Comptroller.sol#L1470

Vulnerability details

Impact

The Comptroller contract is forked from the Compound protocol and the reward token is modified, but the getWETHAddress function still uses the COMP token address

     function getWETHAddress() virtual public view returns (address) {
         return 0xc00e94Cb662C3520282E6f5717214004A7f26888;
     }

Proof of Concept

https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Comptroller.sol#L1470

Tools Used

None

Recommended Mitigation Steps

Modify the return value of getWETHAddress to the weth token address

Lines of code

https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/ERC20.sol#L29-L34

Vulnerability details

Impact

In the ERC20 standard, _totalSupply indicates the number of tokens minted, but the tokens in the _initialSupply parts of the Note contract have not been minted, which causes _totalSupply to be incorrect

Proof of Concept

https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/ERC20.sol#L29-L34 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/ERC20.sol#L237-L247

Tools Used

None

Recommended Mitigation Steps

Consider making _initialSupply == 0

Lines of code

https://github.com/Plex-Engineer/lending-market/blob/755424c1f9ab3f9f0408443e6606f94e4f08a990/contracts/Governance/GovernorBravoDelegate.sol#L63

Vulnerability details

Impact

In the queue function of the GovernorBravoDelegate contract, newProposal.executed = true will be set, which will make state(proposalId) != ProposalState.Queued, so the proposal cannot be executed.

    function execute(uint proposalId) external payable {
        require(state(proposalId) == ProposalState.Queued, "GovernorBravo::execute: proposal can only be executed if it is queued");

Proof of Concept

https://github.com/Plex-Engineer/lending-market/blob/755424c1f9ab3f9f0408443e6606f94e4f08a990/contracts/Governance/GovernorBravoDelegate.sol#L63

Tools Used

None

Recommended Mitigation Steps

Modify the following statements in the queue function

-         newProposal.executed = true;
+        newProposal.executed = false;

Lines of code

https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Note.sol#L13-L19

Vulnerability details

Impact

In Note contract, if _initialSupply ! = 0, _totalSupply will overflow when the _mint_to_Accountant function executes _mint(msg.sender, type(uint).max)

    constructor(string memory name_, string memory symbol_, uint256 totalSupply_) public {
        _name = name_;
        _symbol = symbol_;
	    _initialSupply = totalSupply_;
	    _totalSupply = totalSupply_;
    }
...
    function _mint(address account, uint256 amount) internal   {
        require(account != address(0), "ERC20: mint to the zero address");

        _beforeTokenTransfer(address(0), account, amount);

        _totalSupply += amount;
        _balances[account] += amount;
        emit Transfer(address(0), account, amount);

        _afterTokenTransfer(address(0), account, amount);
    }

Proof of Concept

https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Note.sol#L13-L19 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/ERC20.sol#L29-L34 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/ERC20.sol#L237-L247

Tools Used

None

Recommended Mitigation Steps

ERC20.sol

    constructor(string memory name_, string memory symbol_) public {
        _name = name_;
        _symbol = symbol_;
    }

note.sol

    constructor() ERC20("Note", "NOTE") {
        admin = msg.sender;
    }

Lines of code

https://github.com/Plex-Engineer/lending-market/blob/755424c1f9ab3f9f0408443e6606f94e4f08a990/contracts/Treasury/TreasuryDelegator.sol#L44-L56

Vulnerability details

Impact

In AccountantDelegator and TreasuryDelegator contracts, when using abi.decode(data, (uint)) to convert data to uint type, the length of data is not checked, when the returned data is of bytes type, the abi.decode will return 0x20.

Proof of Concept

https://github.com/Plex-Engineer/lending-market/blob/755424c1f9ab3f9f0408443e6606f94e4f08a990/contracts/Treasury/TreasuryDelegator.sol#L44-L56 https://github.com/Plex-Engineer/lending-market/blob/755424c1f9ab3f9f0408443e6606f94e4f08a990/contracts/Accountant/AccountantDelegator.sol#L54-L74 This contract can test that when the function returns bytes data, abi.encode will decode the return value as 0x20.

pragma solidity 0.8.10;
contract A{
    uint public destination;
    uint256 public number;
    function convertA() external{
        (bool su,bytes memory ret )= address(this).call(abi.encodeWithSelector(this.ret.selector));
        number = ret.length;
        destination = abi.decode(ret, (uint));
    }
    function ret() public returns(bytes memory){
        return "1234";
    }
}

Tools Used

None

Recommended Mitigation Steps

Requires data.length == 32