This document is a security audit report performed by RideSolo, where Aurora Token has been reviewed.
6 issues were reported:
- 1 medium severity issue.
- 5 low severity issues.
As per ERC20 standard "The function SHOULD throw if the message caller’s account balance does not have enough tokens to spend" however the implemented transfer function returns false instead of throwing the transaction if the user doesn't have enough of fund.
In the other hand dapp developers should also be aware that as per ERC20 standard "Callers MUST handle false from returns (bool success). Callers MUST NOT assume that false is never returned!", meaning that transfer function is returning false which it shouldn't but the dapp developers must not assume that false is never returned.
function transfer(address _to, uint256 _value) public returns (bool success) {
if (balances[msg.sender] >= _value && _value > 0) {
balances[msg.sender] -= _value;
balances[_to] += _value;
Transfer(msg.sender, _to, _value);
return true;
} else { return false; }
}
function transferFrom(address _from, address _to, uint256 _value) public returns (bool success) {
//same as above. Replace this line with the following if you want to protect against wrapping uints.
//if (balances[_from] >= _value && allowed[_from][msg.sender] >= _value && balances[_to] + _value > balances[_to]) {
if (balances[_from] >= _value && allowed[_from][msg.sender] >= _value && _value > 0) {
balances[_to] += _value;
balances[_from] -= _value;
allowed[_from][msg.sender] -= _value;
Transfer(_from, _to, _value);
return true;
} else { return false; }
}
As per ERC20 standard "Transfers of 0 values MUST be treated as normal transfers and fire the Transfer event" both transfer and transferFrom ignore transfers of zero value and return false, instead of threating it as normal transfers.
function transfer(address _to, uint256 _value) public returns (bool success) {
if (balances[msg.sender] >= _value && _value > 0) {
balances[msg.sender] -= _value;
balances[_to] += _value;
Transfer(msg.sender, _to, _value);
return true;
} else { return false; }
}
function transferFrom(address _from, address _to, uint256 _value) public returns (bool success) {
//same as above. Replace this line with the following if you want to protect against wrapping uints.
//if (balances[_from] >= _value && allowed[_from][msg.sender] >= _value && balances[_to] + _value > balances[_to]) {
if (balances[_from] >= _value && allowed[_from][msg.sender] >= _value && _value > 0) {
balances[_to] += _value;
balances[_from] -= _value;
allowed[_from][msg.sender] -= _value;
Transfer(_from, _to, _value);
return true;
} else { return false; }
}
Following ERC20 standard transferFrom "function SHOULD throw unless the _from account has deliberately authorized the sender of the message via some mechanism", meaning that the function should not return false when the msg.sender is not allowed to make a transfer.
In the other hand dapp developers should also be aware that as per ERC20 standard "Callers MUST handle false from returns (bool success). Callers MUST NOT assume that false is never returned!", meaning that transferFrom function is returning false which it shouldn't but the dapp developers must not assume that false is never returned.
function transferFrom(address _from, address _to, uint256 _value) public returns (bool success) {
//same as above. Replace this line with the following if you want to protect against wrapping uints.
//if (balances[_from] >= _value && allowed[_from][msg.sender] >= _value && balances[_to] + _value > balances[_to]) {
if (balances[_from] >= _value && allowed[_from][msg.sender] >= _value && _value > 0) {
balances[_to] += _value;
balances[_from] -= _value;
allowed[_from][msg.sender] -= _value;
Transfer(_from, _to, _value);
return true;
} else { return false; }
}
transfer and transferFrom allow the destination address to be equal to zero, meaning that users fund can be lost if sent to it by mistake.
function transfer(address _to, uint256 _value) public returns (bool success) {
if (balances[msg.sender] >= _value && _value > 0) {
balances[msg.sender] -= _value;
balances[_to] += _value;
Transfer(msg.sender, _to, _value);
return true;
} else { return false; }
}
function transferFrom(address _from, address _to, uint256 _value) public returns (bool success) {
//same as above. Replace this line with the following if you want to protect against wrapping uints.
//if (balances[_from] >= _value && allowed[_from][msg.sender] >= _value && balances[_to] + _value > balances[_to]) {
if (balances[_from] >= _value && allowed[_from][msg.sender] >= _value && _value > 0) {
balances[_to] += _value;
balances[_from] -= _value;
allowed[_from][msg.sender] -= _value;
Transfer(_from, _to, _value);
return true;
} else { return false; }
}
Transfer event is not emitted when the initial fund are assigned to the msg.sender inside the contructor.
ERC20: "A token contract which creates new tokens SHOULD trigger a Transfer event with the _from address set to 0x0 when tokens are created."
function ArthurStandardToken(
uint256 _initialAmount,
string _tokenName,
uint8 _decimalUnits,
string _tokenSymbol
) public {
balances[msg.sender] = _initialAmount; // Give the creator all initial tokens
totalSupply = _initialAmount; // Update total supply
name = _tokenName; // Set the name for display purposes
decimals = _decimalUnits; // Amount of decimals for display purposes
symbol = _tokenSymbol; // Set the symbol for display purposes
}
- It is possible to double withdrawal attack. More details here
- Lack of transaction handling mechanism issue. WARNING! This is a very common issue and it already caused millions of dollars losses for lots of token users! More details here
ERC20 Compliannce issues should be solved