code-423n4 / 2024-07-reserve-validation

0 stars 0 forks source link

Block Timestamp Manipulation in permit Function on StRSR contract #111

Closed c4-bot-7 closed 1 month ago

c4-bot-7 commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/StRSR.sol#L922-L940 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/StRSR.sol#L931

Vulnerability details

Impact

Detailed description of the impact of this finding.

Block Timestamp Manipulation in permit Function on StRSR contract

Contract: StRSRP1 Function Name: permit(address owner, address spender, uint256 value, uint256 deadline, uint8 v, bytes32 r, bytes32 s) PC Address: 11406 Issue Type: Block Timestamp Manipulation Estimated Gas Usage: 1672 - 1767

The permit function of the StRSRP1 contract uses the block.timestamp environment variable to determine whether a permit has expired:

require(block.timestamp <= deadline, "ERC20Permit: expired deadline");

However, block.timestamp can be influenced by a malicious miner.

While the miner cannot drastically change the block timestamp, they can adjust it slightly within a permissible range, which could be enough to exploit this condition.

This introduces a potential vulnerability where a malicious miner could manipulate the timestamp to either prematurely expire a permit or extend its validity beyond the intended deadline.

A miner could potentially manipulate the block timestamp, allowing them to alter the validity period of a permit. This could lead to:

Early expiration: Preventing a legitimate user from executing a permit operation even though it was intended to be valid.

Extended validity: Allowing a permit to be used after it was supposed to have expired, potentially enabling unauthorised transactions.

Use Case A similar issue(s):

https://owasp.org/www-project-smart-contract-top-10/2023/en/src/SC03-timestamp-dependence.html

https://medium.com/@zhongqiangc/exploiting-fomo3d-family-part-2-b792fc1fb3f3

Fomo3D was a popular Ethereum-based game that heavily relied on the block timestamp for game mechanics. 

The game was essentially a lottery where participants could buy keys, and the game would end when a timer ran out. 

The last person to buy a key before the timer expired would win the entire pot.

https://github.com/code-423n4/2022-10-holograph-findings/issues/427#issuecomment-1308894416

HolographOperator.sol#L491-L511

Using block.number and block.timestamp as a source of randomness is commonly advised against, as the outcome can be manipulated by calling contracts. 

In this case a compromised layer-zero-endpoint would be able to retry the selection of the primary operator until the result is favourable to the malicious actor.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

A malicious miner could manipulate the block timestamp as follows:

// Example scenario where a permit's deadline is manipulated
uint256 originalTimestamp = block.timestamp; // Assume the block timestamp is initially 1000
uint256 manipulatedTimestamp = originalTimestamp + 10; // Miner slightly increases the timestamp

// If the deadline was exactly 1000, the permit would still be valid even though it should have expired.
require(manipulatedTimestamp <= 1000, "ERC20Permit: expired deadline");

MythX logs

--------------------
In file: /2024-07-reserve/contracts/p1/StRSR.sol:931

require(block.timestamp <= deadline, "ERC20Permit: expired deadline")

--------------------
Initial State:

Account: [CREATOR], balance: 0x0, nonce:0, storage:{}
Account: [ATTACKER], balance: 0x0, nonce:0, storage:{}

Transaction Sequence:

Caller: [CREATOR], calldata: , decoded_data: , value: 0x0
Caller: [ATTACKER], function: permit(address,address,uint256,uint256,uint8,bytes32,bytes32), txdata: 0xd505accf0000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000, decoded_data: ('0x0000000000000000000000000000000000000040', '0x0000000000000000000000000000000000000000', 0, 0, 0, '\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00', '\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'), value: 0x0

This scenario shows how the validity check could be bypassed or incorrectly fail due to miner manipulation.

POC - Remix IDE:

  1. Deploy to address(0x5B3...)
  2. Call permit from address(0xAb8...)
    StRSR.permit(0x0000000000000000000000000000000000000040,
    0x0000000000000000000000000000000000000000,
    0,
    0,
    0,
    0x0000000000000000000000000000000000000000000000000000000000000000,
    0x0000000000000000000000000000000000000000000000000000000000000000)
    transact to StRSRP1.permit pending ... 
    [vm]from: 0xAb8...35cb2to: IAsset.(fallback) 0x5B3...eddC4value: 1000000000000000000 weidata: 0xd50...00000logs: 0hash: 0x9fa...fa7ba
    status  0x1 Transaction mined and execution succeed
    transaction hash    0x9fa826008c41792bc41725717843ba4bc47c292f5dd5c3a2a03710dd6e4fa7ba
    block hash  0xad1e74a741bcc3654e83cbc8aafbc88428e54c302687c6a8dc69cf80c746e3f7
    block number    40
    from    0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2
    to  IAsset.(fallback) 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4
    gas 25268 gas
    transaction cost    21972 gas 
    input   0xd50...00000
    decoded input   -
    decoded output   - 
    logs    []
    raw logs    []
    value   1000000000000000000 wei

Tools Used

Manual review and MythX and Remix IDE.

Recommended Mitigation Steps

To protect against miners' manipulation of block.timestamp, it's better to use and oracle instead: require(currentTime <= deadline, "ERC20Permit: expired deadline");.

interface ITimeOracle {
    function getCurrentTime() external returns (uint256);
}

contract StRSRP1 {
    ITimeOracle public timeOracle;

    constructor(address oracleAddress) {
        timeOracle = ITimeOracle(oracleAddress);
    }

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

        uint256 currentTime = timeOracle.getCurrentTime();
        require(currentTime <= deadline, "ERC20Permit: expired deadline");

        bytes32 structHash = keccak256(
            abi.encode(_PERMIT_TYPEHASH, owner, spender, value, _useNonce(owner), deadline)
        );

        PermitLib.requireSignature(owner, _hashTypedDataV4(structHash), v, r, s);

        _approve(owner, spender, value);
    }  
}

Assessed type

Timing