Skip to content
This repository has been archived by the owner on May 26, 2023. It is now read-only.

Check the integrity of balances #143

Closed
wants to merge 1 commit into from

Conversation

luongnt95
Copy link
Contributor

@luongnt95 luongnt95 commented Jul 14, 2017

This PR checks if the amount of balance transfered by the sender and the amount of balance received by the receiver are the same. The below is the contract being used to test the PR. This contract contains a bug in the line balances[msg.sender] -= 2; so it is expected to display "ERC20 bug" in the result.

contract ERC20 {
  // Balances for each account
  mapping(address => uint256) balances;
  bytes32 public diff;

  function transfer(address _to, uint256 _amount){
    uint256 prev_sender = balances[msg.sender];
    uint256 prev_to = balances[_to];
    if (balances[msg.sender] >= _amount && _amount > 0 && balances[_to] + _amount > balances[_to]) {
      balances[msg.sender] -= (_amount + 2);
      balances[_to] += (_amount + 3);
      if (sha3(msg.data) < diff) {
        balances[msg.sender] -= 2;
        balances[_to] -= 3;
        assert(prev_sender - _amount == balances[msg.sender] && prev_to + _amount == balances[_to]);
      }
    }
  }
}

The method that I used to detect the bug is that I use assertion to detect if the condition of the assertion satisfies just by checking if the program would jump into INVALID or not. Because our program uses symbolic values, there's no way for our program to understand that these two lines

balances[msg.sender] -= (_amount + 2);
balances[msg.sender] -= 2;

use the same variable which share a key in the storage. Based on the layout of mapping variables in storage, I tried to simulate the process of storing and loading a mapping variable like balances. The pattern to load or store the key of a mapping variable in storage is something like: MSTORE -> SHA3 -> SLOAD

@luongnt95 luongnt95 requested a review from inian July 14, 2017 09:10
symExec.py Outdated
stack.pop(0)
s0 = stack.pop(0)
s1 = stack.pop(0)
tmp = [str(x) for x in memory[s0: s0 + s1]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s0 and s1 may be symbolic - that case should be handled here right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. This PR is not complete. This SHA3 works with my tests when it's used to find a key in the storage. Other SHA3 should be handled differently as s0 and s1 could be symbolic

@luongnt95 luongnt95 force-pushed the erc20 branch 4 times, most recently from 5123973 to 75db8fc Compare July 14, 2017 14:30
@luongnt95 luongnt95 requested review from inian and loiluu July 14, 2017 14:31
@luongnt95 luongnt95 force-pushed the erc20 branch 2 times, most recently from f360b60 to 6e36815 Compare July 15, 2017 09:03
@loiluu
Copy link
Contributor

loiluu commented Jul 17, 2017

Do consider the overlapping with this PR #144 !

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants