sherlock-audit / 2022-10-mover-judging

1 stars 0 forks source link

GalloDaSballo - M-03 Blockhash doesn't work for current block #50

Closed sherlock-admin closed 2 years ago

sherlock-admin commented 2 years ago

GalloDaSballo

medium

M-03 Blockhash doesn't work for current block

Summary

https://github.com/sherlock-audit/2022-10-mover/blob/main/cardtopup_contract/contracts/HardenedTopupProxy.sol#L1058-L1059

Should check that block provided is less than current as the current blockHash cannot be known

Vulnerability Detail

Checking for blockhash(block.number) will always return 0

Impact

Am not sure value can be stolen via this

Code Snippet

// SPDX-License-Identifier: MIT
pragma solidity 0.8.10;

contract MoverTests {
  function prooveBlockhash(uint256 blockNumberDelta, bytes32 expected) external {
    uint256 blockTarget = block.number - blockNumberDelta;
    require(expected == blockhash(blockTarget), "Hash is not expected");
  }
}

If the value was non-zero we'd get a revert

>>> c.prooveBlockhash(0, 0, {"from": a[0]})
Transaction sent: 0x3a9d4c6aab7c94651a2176fdd75ea3ead6cc7f94408b34bfa480a26f090d5769
  Gas price: 0.0 gwei   Gas limit: 12000000   Nonce: 7
  MoverTests.prooveBlockhash confirmed   Block: 15820123   Gas used: 21665 (0.18%)

<Transaction '0x3a9d4c6aab7c94651a2176fdd75ea3ead6cc7f94408b34bfa480a26f090d5769'>
>>>

However we do not, meaning a 0 blockhash will be provided

Tool used

Manual Review

Recommendation

Prevent using current block

McMannaman commented 2 years ago

Not an issue, works as expected. The approve proof has to already be in block (meaning at least one block before current). For current block blockhash would be returned as 0 since it's not finalized, and this would result in the failure of checks and revert of tx.