This document is a security audit report performed by danbogd, where Tether USD has been reviewed.
Сommit hash .
In total, 9 issues were reported including:
- 0 medium severity issues
- 4 low severity issues
- 5 owner privileges (ability of owner to manipulate contract, may be risky for investors)..
- 0 notes.
No critical security issues were found.
-
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.
Add into a function transfer(address _to, ... )
following code:
require( _to != address(this) );
Contract owner allow himself to:
- set to zero balance of any user using addBlackList and destroyBlackFunds mechanism.
Line: 294.
function destroyBlackFunds (address _blackListedUser) public onlyOwner {
require(isBlackListed[_blackListedUser]);
uint dirtyFunds = balanceOf(_blackListedUser);
balances[_blackListedUser] = 0;
_totalSupply -= dirtyFunds;
DestroyedBlackFunds(_blackListedUser, dirtyFunds);
}
- pause/unpaused transfer functions;
Line: 238
modifier whenNotPaused() { require(!paused); _; }
- Owner can upgrade contract and implement any logic in the new contract. And even if the new contract will be audited, at any time possible to change the address of the new contract again to not audited and insecure.
Line: 387.
function deprecate(address _upgradedAddress) public onlyOwner {
deprecated = true;
upgradedAddress = _upgradedAddress;
Deprecate(_upgradedAddress);
}
- can issue any amount of tokens (over totalSupply).
Line: 406.
function issue(uint amount) public onlyOwner {
require(_totalSupply + amount > _totalSupply);
require(balances[owner] + amount > balances[owner]);
balances[owner] += amount;
_totalSupply += amount;
Issue(amount);
}
- change at any time parameters of Maxfee and asisPointsRate.
Line: 429.
function setParams(uint newBasisPoints, uint newMaxFee) public onlyOwner {
require(newBasisPoints < 20);
require(newMaxFee < 50);
basisPointsRate = newBasisPoints;
maximumFee = newMaxFee.mul(10**decimals);
Params(basisPointsRate, maximumFee);
}
There is no way to reset approved value to 0, because approve function contains:
require(!((_value != 0) && (allowed[msg.sender][_spender] != 0))); throw
Also it breaks the EIP20 security recommendation:
To prevent attack vectors like the one described [here](https://docs.google.com/document/d/1YLPtQxZu1UAvO9cZ1O2RPXBbT0mooh4DYKjA_jp-RLM/edit) and discussed [here](https://github.com/ethereum/EIPs/issues/20#issuecomment-263524729), clients SHOULD make sure to create user interfaces in such a way that they set the allowance first to 0 before setting it to another value for the same spender. THOUGH The contract itself shouldn't enforce it, to allow backwards compatibility with contracts deployed before
Line: 205
onlyPayloadSize modifier is workaround to avoid the Short Address Attack. But it doesn't work properly.
This method failed to execute when it was called from Parity multisignature wallet. The EVM pads call from this multisignature wallet, making the total 96 bytes instead of the expected 68.
If transfer and transferFrom are used by a subcontract function with fewer arguments, the onlyPayloadSize check will fail. It is not possible to adapt the workaround to prevent this issue.
More details here.
modifier onlyPayloadSize(uint size) {
require(!(msg.data.length < size + 4));
_;
}
Remove this workaround.
According to ERC20 standard when coins are minted a Transfer event should be emitted.
Line 335.
The review did not show any critical issues, some of low severity issues were found.