Ethos Unaffected by batchOverflow ERC20 Bug

Hey Ethos community! Doing a special post here today in light of recent news of a vulnerability in some ERC20 contracts. Specifically, this bug is the batchOverflow bug outlined in this piece: https://medium.com/coinmonks/alert-new-batchoverflow-bug-in-multiple-erc20-smart-contracts-cve-2018-10299-511067db6536

All credit goes to them for finding this bug. I’ll be trying to simplify it to make it understandable and address any concerns you all may have around this bug.

Contract/Transaction in Question

Contract with the vulnerability: https://etherscan.io/address/0xc5d105e63711398af9bbff092d4b6769c82f793d#code
Transaction exploiting vulnerability: https://etherscan.io/tx/0xad89ff16fd1ebe3a0a7cf4ed282302c06626c1af33221ebe0d3a470aba4a660f

Breaking it Down — Integer Overflow

So what’s going on here? How did they send so many tokens? It essentially boils down to the age-old “integer overflow” attack. An integer overflow occurs when you try to enter a value that is too big for the range that can be stored.

For example, if you have an 8-bit integer, the largest number you can represent is 255 or 0b11111111 in binary. If you add 1 to 0b11111111, instead of getting 256 (0b100000000), which requires at least one more bit to store, you will get 0 as all the bits will flip to 0. If you notice, 256 has a 1 then 8 0s, but in an integer overflow, only the 0s are stored. For more information on integer overflows, read here: https://en.wikipedia.org/wiki/Integer_overflow

How Did That Cause a Vulnerability?

Let’s take a look at the code…

function batchTransfer(address[] _receivers, uint256 _value) public      whenNotPaused returns (bool) {
uint cnt = _receivers.length;
uint256 amount = uint256(cnt) * _value;
require(cnt > 0 && cnt <= 20);
require(_value > 0 && balances[msg.sender] >= amount);
     balances[msg.sender] = balances[msg.sender].sub(amount);
for (uint i = 0; i < cnt; i++) {
balances[_receivers[i]] = balances[_receivers[i]].add(_value);
Transfer(msg.sender, _receivers[i], _value);
}
return true;
}
}

And let’s take a look at the offending transaction data

Function: batchTransfer(address[] _receivers, uint256 _value)
MethodID: 0x83f12fec
[0]: 0000000000000000000000000000000000000000000000000000000000000040
[1]: 8000000000000000000000000000000000000000000000000000000000000000
[2]: 0000000000000000000000000000000000000000000000000000000000000002
[3]: 000000000000000000000000b4d30cac5124b46c2df0cf3e3e1be05f42119033
[4]: 0000000000000000000000000e823ffe018727585eaf5bc769fa80472f76c3d7

The attacker used a very large value for the uint256 _value (specifically 0x8000000000000000000000000000000000000000000000000000000000000000) as seen in [1] and then they passed in two receivers (0xb4d30cac5124b46c2df0cf3e3e1be05f42119033) and (0x0e823ffe018727585eaf5bc769fa80472f76c3d7) as seen in [3] and [4]

uint cnt = _receivers.length;
uint256 amount = uint256(cnt) * _value;

Those two lines, with cnt = 2 and _value equal to 0x8000000000000000000000000000000000000000000000000000000000000000, cause the amount to overflow and zero out which leads us to the first issue in this contract.

Always use SafeMath

The following is a SafeMath implementation

/**
* Math operations with safety checks
*/
contract SafeMath {
function safeMul(uint a, uint b) internal returns (uint) {
uint c = a * b;
assert(a == 0 || c / a == b);
return c;
}
function safeDiv(uint a, uint b) internal returns (uint) {
assert(b > 0);
uint c = a / b;
assert(a == b * c + a % b);
return c;
}
function safeSub(uint a, uint b) internal returns (uint) {
assert(b <= a);
return a - b;
}
function safeAdd(uint a, uint b) internal returns (uint) {
uint c = a + b;
assert(c>=a && c>=b);
return c;
}
function max64(uint64 a, uint64 b) internal constant returns (uint64) {
return a >= b ? a : b;
}
function min64(uint64 a, uint64 b) internal constant returns (uint64) {
return a < b ? a : b;
}
function max256(uint256 a, uint256 b) internal constant returns (uint256) {
return a >= b ? a : b;
}
function min256(uint256 a, uint256 b) internal constant returns (uint256) {
return a < b ? a : b;
}
function assert(bool assertion) internal {
if (!assertion) {
throw;
}
}
}

SafeMath prevents integer overflow attacks by implementing “safe” versions of addition, subtraction etc.

While the contract actually did include the SafeMath library at the top of the contract, they didn’t use it here. It is also worth noting that above is the ETHOS smart contract SafeMath implementation and that the contract in question had a different implementation. I did not do an in depth audit of the contract so I cannot attest to the accuracy of their implementation.

Sanity Checks didn’t check for Sane Input

require(cnt > 0 && cnt <= 20);
require(_value > 0 && balances[msg.sender] >= amount);

The next few lines were designed to be sanity checks, but they didn’t actually check for the overflow that just happened. cnt = 2 which is well within the range and the _value is > 0 while the amount got overflowed and zeroed out.

Woohoo Free Money!

balances[msg.sender] = balances[msg.sender].sub(amounatt);
for (uint i = 0; i < cnt; i++) {
balances[_receivers[i]] = balances[_receivers[i]].add(_value);
Transfer(msg.sender, _receivers[i], _value);
}

The next few lines, after passing the “sanity check”, execute the transaction sending the massive _value that was passed in while subtracting nothing from the attacker.

Some Observations

The contract makes a number of errors. Most notably, they implemented SafeMath, but didn’t use it. The batchTransfer function also is not part of the standard ERC20 interface, although there is nothing wrong with it in principle. Implementing a new function can sometimes be dangerous since it won’t be as battle-tested and audited as standard ERC20 functions which are already well understood.

ETHOS Contract Unaffected by batchTransfer Vulnerability

The ETHOS smart contract implements SafeMath (as outlined above) and uses SafeMath for every mathematical operation. While this does consume slightly more gas, the security that is gained by eliminating potential overflow attacks is considerable.

ETHOS also does NOT implement the batchTransfer function in the token contract. It is not part of the ERC20 standard interface. ETHOS has developed a tool internally that will be used for airdrops and payroll that has similar and greater functionality than the batchTransfer function which has been separated out into an independent contract for greater security.

The ETHOS token contract also went through an internal security audit whose results were published here: https://www.ethos.io/2017/08/08/bqx-smart-contract-code-review-result/

Bottom line, batchTransfer is a vulnerability that only affects a subset of ERC20 contracts that implemented an additional function outside of the standard ERC20 interface and ETHOS is unaffected by this vulnerability.

Shingo Lavine
Founder & CEO
Ethos.io


Ethos Unaffected by batchOverflow ERC20 Bug was originally published in Ethos.io on Medium, where people are continuing the conversation by highlighting and responding to this story.