ZhangZhuoSJTU / Web3Bugs

Demystifying Exploitable Bugs in Smart Contracts
1.53k stars 207 forks source link

Figure 6, discrepancy #12

Closed baranyak closed 1 year ago

baranyak commented 1 year ago

Hi, I am really sorry, if I am confusing something. But I believe you should have the opposite check in the Vote contract example, Figure 6:

contract Vote{
    struct Proposal {
...
    function vote(uint amount) external {
        require(proposal.sTime + 2 days > block.timestamp, "voting has ended"); // should not it be opposite "<"
ZhangZhuoSJTU commented 1 year ago

Hi @baranyak

Thank you for bringing up this discussion. Please don't hesitate to raise any concerns or questions you may have, as we appreciate your input and strive to improve the quality of our paper.

Basically, the code in Figure 6 is shown as follows.

contract Vote {
  struct Proposal {
    uint160 sTime; address newOwner; 
  }
  IERC20 votingToken;
  address owner;  
  Proposal proposal;

  function propose() external {
    require(proposal.sTime == 0, "on-going proposal");
    proposal = Proposal(block.timestamp, msg.sender);
  }
  function vote(uint amount) external {
    require(proposal.sTime + 2 days > block.timestamp,
      "voting has ended");
    votingToken.transferFrom(
      msg.sender, address(this), amount);
  }
  function end() external {
    require(proposal.sTime != 0, "no proposal");
    require(proposal.sTime + 2 days < block.timestamp,
      "voting has not ended");
    require(votingToken.balanceOf(address(this))*2 > 
        votingToken.totalSupply(), "vote failed");
    owner = proposal.newOwner;
    delete proposal;
  }
  function getLockedFunds() external onlyOwner { ... }
}

One of the essential security features of this contract is that the vote function is designed to only execute during a 2-day window after the proposal is proposed, while the end function can only be invoked after 2 days have passed. This ensures that the proposal is open for voting for a limited time and that the voting period has ended before the proposal is executed.

  function vote(uint amount) external {
    require(proposal.sTime + 2 days > block.timestamp,
      "voting has ended");

Regarding the above code snippet, the > operator ensures that the current block.timestamp falls within the 2-day window after the proposal is proposed.

Consider a scenario where we waited for 10 years, and the block.timestamp became an extremely large number. In such a case, the require(proposal.sTime + 2 days > block.timestamp) condition would not be satisfied. Such a behavior aligns with the original design of the contract, which aims to limit the voting period to 2 days after the proposal creation time.

Please feel free to let me know if you have any further concerns or questions. I am happy to discuss these topics with you and provide any additional information or clarifications that you may need.

baranyak commented 1 year ago

Thank you very much for the clarification and for the great paper. Closing the issue.