Frankencoin - karanctf's results

Lines of code

https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Equity.sol#L313

Vulnerability details

Impact

restructureCapTable function in Equity.sol burns only first element of helpers array

Proof of Concept

in below code snipet address current = addressesToWipe[0]; makes this code only burn the balance of first element of helpers.

  function restructureCapTable(address[] calldata helpers, address[] calldata addressesToWipe) public {
        require(zchf.equity() < MINIMUM_EQUITY);
        checkQualified(msg.sender, helpers);
        for (uint256 i = 0; i<addressesToWipe.length; i++){
            address current = addressesToWipe[0];//@audit
            _burn(current, balanceOf(current));
        }
    }

https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Equity.sol#L313

Tools Used

Manual review

Recommended Mitigation Steps

replace 0 with i so it burns balance of all helpers

  function restructureCapTable(address[] calldata helpers, address[] calldata addressesToWipe) public {
        require(zchf.equity() < MINIMUM_EQUITY);
        checkQualified(msg.sender, helpers);
        for (uint256 i = 0; i<addressesToWipe.length; i++){
--            address current = addressesToWipe[0];//@audit
++            address current = addressesToWipe[i];

            _burn(current, balanceOf(current));
        }
    }

[L-1] canRedeem function can be bypassed

canRedeem function in equity.sol can be bypassed if the paremeter is set to an address that is not insde voteAnchor mapping canRedeem is used in redeem function which returns true if msg.sender is not in voteAnchor

function canRedeem(address owner) public view returns (bool) {
++ require(voteAnchor[owner] > 0);
return anchorTime() - voteAnchor[owner] >= MIN_HOLDING_DURATION;//@audit confusing user
}

This will return true but user can't redeem this will confuse user. Mitigation: Add a check is voteanchor[owner] > 0

[NC-1] OnlyOwner modifier calling another function just to check if msg.sender is owner

consider checking it in modifier itself in ownable.sol

-- function requireOwner(address sender) internal view {
-- if (owner != sender) revert NotOwner();
-- }//@audit qa and gas not need fo rthis function can be directly used

modifier onlyOwner() {
require(msg.sender == owner);
-- requireOwner(msg.sender);
_;
}

[NC-2] Inconsitency in grouping numbers

In the code base in multiple places grouping large number like 1000_000 but it should be gouped in thausands which is 1_000_000

uint256 reward = (volume * CHALLENGER_REWARD) / 1000_000;

[NC-3] Check for address 0 in begaining insted of last in permit function

function permit(
address owner,
address spender,
uint256 value,
uint256 deadline,
uint8 v,
bytes32 r,
bytes32 s
) public {

require(deadline >= block.timestamp, "PERMIT_DEADLINE_EXPIRED");
++ require(owner != address(0));
address recoveredAddress = ecrecover(
.
.

);
-- require(recoveredAddress != address(0) && recoveredAddress == owner, "INVALID_SIGNER");//@audit qa check for owner==address(0) in begaining
++ require(recoveredAddress == owner, "INVALID_SIGNER");//@audit qa check for owner==address(0) in begaining
_approve(recoveredAddress, spender, value);
}
}

[G-1] Cache totalSuply() in frankencoin.sol's suggestMinter function https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Frankencoin.sol#L84

if (_applicationPeriod < MIN_APPLICATION_PERIOD && totalSupply() > 0) revert PeriodTooShort();

if (_applicationFee < MIN_FEE && totalSupply() > 0) revert FeeTooLow();// @audit gas for totalsuply

[G-2] In isMinter and returnColatral function cache minters[_minter] https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Frankencoin.sol#L293

function isMinter(address _minter) override public view returns (bool){

return minters[_minter] != 0 && block.timestamp >= minters[_minter];//@audit cache minter gas
}

[G-3] challage.size and challenge.challenger is used 3 times which is state vairable. https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/MintingHub.sol#L287