Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UNUS SED LEO (LEO) #331

Closed
carlossampol opened this issue Jul 10, 2019 · 13 comments

Comments

@carlossampol
Copy link

commented Jul 10, 2019

Audit request

Audit Top 200 CoinMarketCap tokens.

UNUS SED LEO (LEO)

https://www.bitfinex.com/

Source code

Disclosure policy

public

Platform

Ethereum

Number of lines:

339

@yuriy77k yuriy77k changed the title Order No. 1562789343255 UNUS SED LEO (LEO) Jul 10, 2019

@MrCrambo

This comment has been minimized.

Copy link

commented Jul 10, 2019

Auditing time 1 day

@yuriy77k

This comment has been minimized.

Copy link
Member

commented Jul 10, 2019

@MrCrambo assigned

@MrCrambo

This comment has been minimized.

Copy link

commented Jul 11, 2019

My report is finished

@Dexaran

This comment has been minimized.

Copy link
Member

commented Jul 12, 2019

@yuriy77k assign me.
Auditing time 4 days.

@gorbunovperm

This comment has been minimized.

Copy link

commented Jul 12, 2019

@Dexaran Wow, that's cool!

@yuriy77k

This comment has been minimized.

Copy link
Member

commented Jul 12, 2019

@Dexaran assigned.

@Dexaran

This comment has been minimized.

Copy link
Member

commented Jul 15, 2019

My report is finished.

@danbogd

This comment has been minimized.

Copy link

commented Jul 18, 2019

Auditing time: 3 days.

@yuriy77k

This comment has been minimized.

Copy link
Member

commented Jul 18, 2019

@danbogd assigned

@danbogd

This comment has been minimized.

Copy link

commented Jul 20, 2019

My report is finished.

@yuriy77k

This comment has been minimized.

Copy link
Member

commented Jul 20, 2019

UNUS SED LEO token Security Audit Report

1. Summary

UNUS SED LEO token smart contract security audit report performed by Callisto Security Audit Department

2. In scope

3. Findings

In total, 7 issues were reported including:

  • 3 low severity issues.

  • 2 owner privileges (the ability of an owner to manipulate contract, may be risky for investors).

  • 2 minor observation / non-security issue

No critical security issues were found.

3.1 proxyPayment function syntax

Severity: minor observation / non-security issue

Code snippet

    function proxyPayment(address _owner) public payable returns(bool allowed) {
        allowed = false;
    }

Description

Use of different syntax for returning functions.

Recommendation

It is recommended to use the same syntax for all code fragments in order to facilitate readability. If the other fragments use the return false; syntax , then it is recommended to use the same syntax in this function too.

3.2 Balances in the future

Severity: Low

Code snippet

    function balanceOfAt(address _owner, uint _blockNumber) public view
        returns (uint) {
        if ((balances[_owner].length == 0)
            || (balances[_owner][0].fromBlock > _blockNumber)) {
            if (address(parentToken) != address(0)) {
                return parentToken.balanceOfAt(_owner, min(_blockNumber, parentSnapShotBlock));
            } else {
                return 0;
            }
        } else {
            return getValueAt(balances[_owner], _blockNumber);
        }
    }
    function totalSupplyAt(uint _blockNumber) public view returns(uint) {
        if ((totalSupplyHistory.length == 0)
            || (totalSupplyHistory[0].fromBlock > _blockNumber)) {
            if (address(parentToken) != address(0)) {
                return parentToken.totalSupplyAt(min(_blockNumber, parentSnapShotBlock));
            } else {
                return 0;
            }
        } else {
            return getValueAt(totalSupplyHistory, _blockNumber);
        }
    }
    function getValueAt(Checkpoint[] storage checkpoints, uint _block
    ) view internal returns (uint) {
        if (checkpoints.length == 0) return 0;
        if (_block >= checkpoints[checkpoints.length-1].fromBlock)
            return checkpoints[checkpoints.length-1].value;
        if (_block < checkpoints[0].fromBlock) return 0;

        // Binary search of the value in the array
        uint min = 0;
        uint max = checkpoints.length-1;
        uint mid = 0;
        while (max > min) {
            mid = (max + min + 1)/ 2;
            if (checkpoints[mid].fromBlock<=_block) {
                min = mid;
            } else {
                max = mid-1;
            }
        }
        return checkpoints[min].value;
    }

Description

Balances may be requested for blocks that have not yet been mined. In chis case the contract will return the actual balance of the queried address while it is not a correct information because the contract does not know the balance of the queried address in the future.

This can not harm the LEO token contract, but this may confuse third party services that work with totalSupplyAt or balanceOfAt functions.

3.3 Void receiveApproval function

Severity: Low

Code snippet

    function approveAndCall(address _spender, uint256 _amount, bytes memory _extraData
    ) public returns (bool success) {
        require(approve(_spender, _amount));

        ApproveAndCallFallBack(_spender).receiveApproval(
            msg.sender,
            _amount,
            address(this),
            _extraData
        );

        return true;
    }
contract ApproveAndCallFallBack {
    function receiveApproval(address from, uint256 _amount, address _token, bytes memory _data) public;
}

Description

It would be better if the receiveApproval function returns true on successful execution. Otherwise, permissive fallback function may handle the receiveApproval invocation without causing the transaction to fail even if the receiver contract does not implement receiveApproval function.

3.4 TokenFactory is not upgradeable

Severity: minor observation / non-security issue

Description

The default minime token is designed to by upgradeable, but the TokenFactory contract which is responsible for the upgrading process does not allow for implementation of new features/functions as it is only deploying the same version of the MinimeToken contract.

The address of TokenFactory can not be updated in the deployed contract.

3.5 Owner may control the emission manually

Severity: Owner privileges / non-security issue

Code snippet

    /// @notice `onlyOwner` can upgrade the controller contract
    /// @param _newControllerAddress The address that will have the token control logic
    function upgradeController(address _newControllerAddress) public onlyOwner {
        tokenContract.changeController(_newControllerAddress);
        emit UpgradedController(_newControllerAddress);
    }
    function generateTokens(address _owner, uint _amount
    ) public onlyController returns (bool) {
        uint curTotalSupply = totalSupply();
        require(curTotalSupply + _amount >= curTotalSupply); // Check for overflow
        uint previousBalanceTo = balanceOf(_owner);
        require(previousBalanceTo + _amount >= previousBalanceTo); // Check for overflow
        updateValueAtNow(totalSupplyHistory, curTotalSupply + _amount);
        updateValueAtNow(balances[_owner], previousBalanceTo + _amount);
        emit Transfer(address(0), _owner, _amount);
        return true;
    }
    function destroyTokens(address _owner, uint _amount
    ) onlyController public returns (bool) {
        uint curTotalSupply = totalSupply();
        require(curTotalSupply >= _amount);
        uint previousBalanceFrom = balanceOf(_owner);
        require(previousBalanceFrom >= _amount);
        updateValueAtNow(totalSupplyHistory, curTotalSupply - _amount);
        updateValueAtNow(balances[_owner], previousBalanceFrom - _amount);
        emit Transfer(_owner, address(0), _amount);
        return true;
    }

Description

Owner can upgrade the controller contract without any restrictions.

Theoretically, the controller contract can be upgraded so that it can mint new tokens or burn any tokens at any address. If this happens, transactions will be available for viewing to everyone.

3.6. Zero address checking

Severity: low

Code snippet

    function changeController(address _newController) public onlyController {
        emit ControlTransferred(controller, _newController);
        controller = _newController;
    }

Description

There are no zero address checking in functions changeController in line 76.

3.7. Owner privileges

Severity: Owner privileges / non-security issue

Description

Owner can disable transfers at any time he wants.

4. Conclusion

The reviewed smart-contract does not contain any medium or high severity security vulnerabilities or other issues. This contract can be used without any serious risk from technical side.

5. Revealing audit reports

https://gist.github.com/Dexaran/eefd2937ee3859f85e9aaaeed2c79a1a

https://gist.github.com/yuriy77k/05b3feee64c2b73be4fdbac74d4421d4

https://gist.github.com/yuriy77k/0772c6a85317d2fce4de2aaff2ea087f

@yuriy77k yuriy77k closed this Jul 20, 2019

@yuriy77k

This comment has been minimized.

Copy link
Member

commented Jul 22, 2019

@danbogd notice about issue 3.1 Out of Gas.

This is not an issue. "Binary search of the value in the array" is used in the code. Even if it will be 2**32 blocks, then it will take only 32 rounds. This can not cause "Out of gas" exception.

@MillianoConti

This comment has been minimized.

Copy link

commented Jul 22, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.