Frankencoin - Proxy's results

Lines of code

https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/PositionFactory.sol#L30-L34 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/PositionFactory.sol#L37-L46

Vulnerability details

Impact

Attacker can steal funds via reorg attack if position is funded within a few block of being created

Proof of Concept

PositionFactory.sol#L30-L34

function clonePosition(address _existing) external returns (address) {
    Position existing = Position(_existing);
    Position clone = Position(createClone(existing.original()));
    return address(clone);
}

The PositionFactory contract clones positions via clonePosition which calls the createClone function.

PositionFactory.sol#L37-L46

function createClone(address target) internal returns (address result) {
    bytes20 targetBytes = bytes20(target);
    assembly {
        let clone := mload(0x40)
        mstore(clone, 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000000000000000000000)
        mstore(add(clone, 0x14), targetBytes)
        mstore(add(clone, 0x28), 0x5af43d82803e903d91602b57fd5bf30000000000000000000000000000000000)
        result := create(0, clone, 0x37)
    }
}

The createClone function uses the create opcode. This means the address of the new position is only dependent on the nonce of the factory. An attacker can abuse this to steal funds via reorg.

Example: Imagine a user creates a position and funds it. This now allows an attacker to steal the funds via a reorg attack. They would maliciously insert their own transaction which they would use to create a clone with their own malicious parameters. This contract would have the same address as the legitimate user's did before the reorg attack. Now the users funds are in the attacker's malicious contract which can be taken.

Tools Used

Manual Review

Recommended Mitigation Steps

Deploy the position contract via the create2 opcode which uses salt.

Example OpenZeppelin cloneDeterministic function

function cloneDeterministic(address implementation, bytes32 salt) internal returns (address instance) {
    /// @solidity memory-safe-assembly
    assembly {
        // Cleans the upper 96 bits of the `implementation` word, then packs the first 3 bytes
        // of the `implementation` address with the bytecode before the address.
        mstore(0x00, or(shr(0xe8, shl(0x60, implementation)), 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000))
        // Packs the remaining 17 bytes of `implementation` with the bytecode after the address.
        mstore(0x20, or(shl(0x78, implementation), 0x5af43d82803e903d91602b57fd5bf3))
        instance := create2(0, 0x09, 0x37, salt)
    }
    require(instance != address(0), "ERC1167: create2 failed");
}

Gas Optimizations

Summary

Total: 73 instances over 4 issues with ~2529 gas saved. Per instance gas savings are listed in the individual issue description.

[G-01] Cheaper Comparison

When comparing integers, it is cheaper to use strict > & < operators over >= & <= operators, even if you must increment or decrement one of the operands. Note: before using this technique, it's important to consider whether incrementing/decrementing one of the operators could result in an over/underflow. This optimization is applicable when the optimizer is turned off.

Example:

function gte() external pure returns (bool) {
    return 2 >= 2;
}

function gtPlusMinusOne() external pure returns (bool) {
    return 2 > 2 - 1;
}

function lte() external pure returns (bool) {
    return 2 <= 2;
}

function ltPlusOne() external pure returns (bool) {
    return 2 < 2 + 1;
}

Saved per instance: 3 gas

Lines:

• StablecoinBridge.sol:50
• StablecoinBridge.sol:51
• Position.sol:53
• Position.sol:110
• Position.sol:184
• Position.sol:305
• Position.sol:307
• Position.sol:374
• Frankencoin.sol:141
• Frankencoin.sol:282
• Frankencoin.sol:294
• MintingHub.sol:109
• MintingHub.sol:171
• MintingHub.sol:172
• MintingHub.sol:201
• MintingHub.sol:218
• MintingHub.sol:255

[G-02] Consider using assembly for math (add, sub, mul, div)

Use assembly for math instead of Solidity. You can check for overflow/underflow in assembly to ensure safety. If using Solidity versions < 0.8.0 and you are using Safemath, you can gain significant gas savings by using assembly to calculate values and checking for overflow/underflow.

Example:

//addition in Solidity
function addTest(uint256 a, uint256 b) public pure {
    uint256 c = a + b;
}

//addition in assembly
function addAssemblyTest(uint256 a, uint256 b) public pure {
    assembly {
        let c := add(a, b)
        if lt(c, a) {
            mstore(0x00, "overflow")
            revert(0x00, 0x20)
        }
    }
}

//subtraction in Solidity
function subTest(uint256 a, uint256 b) public pure {
    uint256 c = a - b;
}

//subtraction in assembly
function subAssemblyTest(uint256 a, uint256 b) public pure {
    assembly {
        let c := sub(a, b)
        if gt(c, a) {
            mstore(0x00, "underflow")
            revert(0x00, 0x20)
        }
    }
}

//multiplication in Solidity
function mulTest(uint256 a, uint256 b) public pure {
    uint256 c = a * b;
}

//multiplication in assembly
function mulAssemblyTest(uint256 a, uint256 b) public pure {
    assembly {
        let c := mul(a, b)
        if lt(c, a) {
            mstore(0x00, "overflow")
            revert(0x00, 0x20)
        }
    }
}

//division in Solidity
function divTest(uint256 a, uint256 b) public pure {
    uint256 c = a * b;
}

//division in assembly
function divAssemblyTest(uint256 a, uint256 b) public pure {
    assembly {
        let c := div(a, b)
        if gt(c, a) {
            mstore(0x00, "underflow")
            revert(0x00, 0x20)
        }
    }
}

Saved per instance:

• add: 40 gas
• sub: 37 gas
• mul: 60 gas
• div: 60 gas

Lines:

• Position.sol:64
• Position.sol:66
• Position.sol:80
• Position.sol:98
• Position.sol:99
• Position.sol:100
• Position.sol:122
• Position.sol:124
• Position.sol:138
• Position.sol:142
• Position.sol:146
• Position.sol:150
• Position.sol:187
• Position.sol:194
• Position.sol:203
• Position.sol:241
• Position.sol:283
• Position.sol:307
• Frankencoin.sol:25
• Frankencoin.sol:88
• Frankencoin.sol:118
• Frankencoin.sol:144
• Frankencoin.sol:166
• Frankencoin.sol:168
• Frankencoin.sol:169
• Frankencoin.sol:196
• Frankencoin.sol:205
• Frankencoin.sol:209
• Frankencoin.sol:227
• Frankencoin.sol:238
• Frankencoin.sol:239
• Frankencoin.sol:253
• Frankencoin.sol:254
• Frankencoin.sol:286
• ERC20.sol:132
• MintingHub.sol:165
• MintingHub.sol:189
• MintingHub.sol:217
• MintingHub.sol:263
• MintingHub.sol:265
• MintingHub.sol:266
• MintingHub.sol:268
• MintingHub.sol:270

[G-04] Consider using assembly to check for address(0)

Example:

contract Contract0 {
    function ownerNotZero(address _addr) public pure {
        require(_addr != address(0), "zero address)");
    }
}

function assemblyOwnerNotZero(address _addr) public pure {
    assembly {
        if iszero(_addr) {
            mstore(0x00, "zero address")
            revert(0x00, 0x20)
        }
    }
}

Saved per instance: 6 gas

Lines:

• ERC20PermitLight.sol:56
• ERC20.sol:152
• ERC20.sol:180

[G-05] Consider using assembly to write storage values

Example:

contract Contract0 {
    address owner = 0xb4c79daB8f259C7Aee6E5b2Aa729821864227e84;

    function updateOwner(address newOwner) public {
        owner = newOwner;
    }
}

contract Contract1 {
    address owner = 0xb4c79daB8f259C7Aee6E5b2Aa729821864227e84;

    function assemblyUpdateOwner(address newOwner) public {
        assembly {
            sstore(owner.slot, newOwner)
        }
    }
}

Saved per instance: 66 gas

Lines:

• Position.sol:57
• Position.sol:65
• Position.sol:67
• Position.sol:80
• Position.sol:82
• Position.sol:112
• Position.sol:143
• Position.sol:165
• Position.sol:205
• Position.sol:272