Lines of code
<https://github.com/code-423n4/2023-04-frankencoin/blob/1022cb106919fba963a89205d3b90bf62543f68f/contracts/MintingHub.sol#L54-L57&gt;
<https://github.com/code-423n4/2023-04-frankencoin/blob/1022cb106919fba963a89205d3b90bf62543f68f/contracts/MintingHub.sol#L124&gt;
<https://github.com/code-423n4/2023-04-frankencoin/blob/1022cb106919fba963a89205d3b90bf62543f68f/contracts/Position.sol#L50-L68&gt;
<https://github.com/code-423n4/2023-04-frankencoin/blob/1022cb106919fba963a89205d3b90bf62543f68f/contracts/Position.sol#L78&gt;
<https://github.com/code-423n4/2023-04-frankencoin/blob/1022cb106919fba963a89205d3b90bf62543f68f/contracts/StablecoinBridge.sol#L26-L31&gt;
<https://github.com/code-423n4/2023-04-frankencoin/blob/1022cb106919fba963a89205d3b90bf62543f68f/contracts/Frankencoin.sol#L125-L128&gt;
<https://github.com/code-423n4/2023-04-frankencoin/blob/1022cb106919fba963a89205d3b90bf62543f68f/contracts/Frankencoin.sol#L83-L89&gt;
<https://github.com/code-423n4/2023-04-frankencoin/blob/1022cb106919fba963a89205d3b90bf62543f68f/contracts/Equity.sol#L93-L95&gt;

Vulnerability details

Impact

User defined address should always have zero address check. This checks SHOULD NOT BE MISSED IN CASE OF A FACTORY CONTRACT.
This will lead to redeployments of contract and blockage of certain functionality as described below.
It is also worth to note that is Data Validation bug was acknowledged by Uniswap v3 and modified. Check here -
<https://github.com/Uniswap/v3-core/blob/main/audits/tob/audit.pdf&gt;

Proof of Concept

In constructor() there is no check for zero address. Once it’s set to any address, there’s no configuration functions to change it.

    constructor(address _owner, address _hub, address _zchf, address _collateral, 
        uint256 _minCollateral, uint256 _initialLimit, uint256 initPeriod, uint256 _duration,
        uint256 _challengePeriod, uint32 _mintingFeePPM, uint256 _liqPrice, uint32 _reservePPM) {
        require(initPeriod >= 3 days); // must be at least three days, recommended to use higher values
        setOwner(_owner);
        original = address(this);
        hub = _hub;
        price = _liqPrice;
        zchf = IFrankencoin(_zchf);
        collateral = IERC20(_collateral);
        mintingFeePPM = _mintingFeePPM;
        reserveContribution = _reservePPM;
        minimumCollateral = _minCollateral;
        challengePeriod = _challengePeriod;
        start = block.timestamp + initPeriod; // one week time to deny the position
        cooldown = start;
        expiration = start + _duration;
        limit = _initialLimit;

Similarly, onTokenTransfer() in StableCoinBridge if constructor address are not set properly and set to address(0), then this function will fail, indefinitely.

    function onTokenTransfer(address from, uint256 amount, bytes calldata) external returns (bool){
        if (msg.sender == address(chf)){
            mintInternal(from, amount);
        } else if (msg.sender == address(zchf)){
            burnInternal(address(this), from, amount);
        } else {
            require(false, "unsupported token");
        }
        return true;
    }

Similarly, no Frankencoin token will be ever transferred to equity.sol as onTokenTransfer() of equity.sol will always fail if constructor is not set correctly. Leading to redeployment of the contract.
Also there is no proper way to update the variable in the same contract.

    function onTokenTransfer(address from, uint256 amount, bytes calldata) external returns (bool) {
        require(msg.sender == address(zchf), "caller must be zchf");
        uint256 equity = zchf.equity();

Tools Used

Manual Review

Recommended Mitigation Steps

• Apply zero address check to all the above mentioned affected code.

The text was updated successfully, but these errors were encountered:

All reactions