Repository

https://github.com/igormuba/EthereumSolidityClasses/tree/master/class20

What Will I Learn?

• Reduce contract creator privileged
• Allow token holders to vote for contract changes
• Introduction to multisig vulnerabilites

Requirements

• Internet connection
• Code editor
• Browser

Difficulty

• Advanced

Tutorial Contents

On the last tutorial I have introduced how can you make a mutable ERC223 smart contract. But at the end of the class I have shown how can the contract "easily" be hacked (if the hacker has access to the creator private keys).

I have also mentioned the moral debate on the regard of mutable smart contracts. Some people support mutable contracts because they allow a relatively easy upgrade in case a bug or vulnerability is found. Some people are against mutable contracts because the owner can be hacked, or the owner might be "evil" and change the contract to make him profit and screw the users somehow.

One of the best solutions, maybe the best one, to fix the vulnerability, is to require a consensus for the changes to take effect. On this tutorial, we will add another piece to our mutable contract, the change manager. We already have a manager to store the variables and balances. The change manager will be responsible for managing the change proposals and votes.

Please, notice that the implementation of the concept made here is basic and allow for an obvious exploit, I will explain the exploit later. The code is very long so I have on the roadmap 2 more classes for the implementation, each one focused on something. This first will be the part where we delegate the responsibility of changing the transfer manager to the change manager. The owner can set the transfer contract in the core and the variable managers only once, after that, only the change manager can do it, with the consensus of the token holders.

On the next tutorial we will delegate the variable manager change to the change manager, and on the last tutorial we will fix the exploit that allows anyone with a small number of tokens to do any change (by default over 50% of the tokens need to approve a change).

The change manager contract

pragma solidity ^0.5.0;

import "browser/ERC223Variables.sol";
import "browser/ERC223Core.sol";

contract ERC223Change{//change manager
}

For now, we will implement only the change proposals for the transfer manager contracts, so, ironically, we only need to import the variables and core contracts, because we need their objects to change the transfer manager on them.

Here are the variables we will need for now and why.

The address of the creator, so the deployer can set once and only once (for the network to work after deployment) the addresses of the other contracts:

    address private _creator;//stores creator address

Core manager variables, for instantiating the object of the core contract and the boolean variable to keep track if it was changed. As said previously, the creator can set the other contracts once and only once, so that the contracts can work. But after being set only the consensus of the token holders can change anything (there is an exploit here, I will explain it later and fix in a future tutorial as by itself it would require a full tutorial):

    ERC223Core private coreManager;//core contract object
    bool private coreManagerSet;//tracks if core has been set

Variables manager, to instantiate the variables manager contract and the boolean to forbid the creator from changing it again once he set (creator can set once and only once):

    ERC223Variables private variablesManager;//variable manager object
    bool private variableManagerSet;//tracks if variable manager has been set

Current ID and the mapping of proposal ID to a proposal struct (explained the very next section):

    uint currentId; //current id of proposal
    mapping(uint=>changeProposal) proposals; //list of all proposals

The proposal structure

On the previous section, you saw the mapping of an ID to a proposal. The proposal structure looks like this:

    struct changeProposal{ //proposal structure
        address newAddress; //new address (code) proposed
        //IDs: 1=transfers 2=variables
        uint contractID; //ID of the type of the contract
        mapping(address=>uint) votes;//tracks votes per address
        uint total;//total of votes for proposal
        bool approved; //tracks if change has over 50% of approval
    }

newAddress is the variable that stores what is the address of the proposed contract. When someone wants to make a change he needs to deploy a contract first and send an address, we expect that token holders read the contract on the address before approving the changes.

contractID records what is the type of the proposed contract, if it will replace a transfer manager or a variables manager. For now, we will work only with transfer manager change proposals, which we are assigning the Id 1.

votes this variable keeps track of how many voters the voter had when he accepted the proposal.

total tracks the total of all the voters and all tokens voted for the new proposal.

approved records the status of the proposal, if it has reached a total of votes superior of 50%+1 of the total supply of tokens, the proposal is accepted.

And here is where the vulnerability is. The fix for it is a bit complex, so I will cover in another tutorial, but I will introduce and explain the problem here.

The exploit

I hope you have noticed the possibility of exploit on the code. The problem is, we record the number of votes (tokens) the user had when he have proposed/approved a change, but this allows him to transfer his tokens to another account and vote from the new account with the same tokens.

This also even allows the total variable to be greater than the total supply of tokens!

However, implementing a fix for this would touch on multisig wallet security (this struct is similar to how multisig works), and that by itself deserves a standalone tutorial because Ethereum security is complex.

To keep the code simple, we will keep this exploit in mind during the whole tutorial, and only after implementing the full decentralized mutable contract we will fix it before deploying the final and complete version.

Constructor and setters

    constructor() public{
        _creator=msg.sender;//sets the deployer as creator
    }
    
    function setCoreManager(address payable newCoreManager)public{//sets core object
        require(msg.sender==_creator&&(!coreManagerSet));//can only be set once
        coreManager=ERC223Core(newCoreManager);//core object pointing to core address
        coreManagerSet=true; //records that the set was already made
    }
    
    function setVariableManager(address newVariableAddress) public{//sets variable manager
        require(msg.sender==_creator&&(!variableManagerSet));//can only be set once(by the creator)
        variablesManager=ERC223Variables(newVariableAddress);//points to the address of the manager
        coreManagerSet=true;//records that it has been set already
    }

The constructor function sets the creator of the contract so that he can tell the contract who are the starting contracts connected to it. But the creator can only do this once, after this, it is required a consensus of the majority of the tokens for any change to be made.

The setCoreManager sets the core contract, but only once. It has to be set by the creator once before we can edit, so the contract know what is the core object and the address of that object, without this the change manager won't be able to find where to call the changer function!

The same thing applies to setVariableManager.

Creating a change proposal

We have already covered and coded the structure of the proposal. Now is time to create the function that will generate the change proposal.

The function is quite big, so, first I will show the whole thing, and then explain every bit so you can understand what we are doing here:

    //creates a proposal for change
    function transferChangeProposal(address newTransferManager) public returns (uint thisProposalId){
        uint myVote = variablesManager.balanceOf(msg.sender);//how many tokens (votes) I own
        changeProposal memory transferChange;//proposal object (from struct)
        proposals[currentId]=transferChange;//stores the proposal object at curernt id
        uint _currentId=currentId;//current id on a new variable for future use
        currentId++;//increases the current ID
        proposals[_currentId].newAddress=newTransferManager;//address for the new contract
        proposals[_currentId].contractID=1;//contract type is transfer(ID=1)
        proposals[_currentId].votes[msg.sender]=myVote;//stores I have voted with my tokens
        proposals[_currentId].total=myVote;//starting votes is my balance
        if(myVote>((variablesManager.totalSupply()/2)+1)){//if my balance is more than 50%+1 of supply
            proposals[_currentId].approved=true;//the proposal is approved
            coreManager.setTransferManager(newTransferManager);//new transfer is updated on the core
            variablesManager.setTransferManager(newTransferManager);//new transfer is updated on variable manager
        }else{
            proposals[_currentId].approved=false; //if I have less than enough, proposal is not yet approved
        }
        thisProposalId=_currentId; //returns the ID of the proposal
    }

function transferChangeProposal(address newTransferManager) public returns (uint thisProposalId) Receives as an argument the new address of the new contract we want the code to replace the old one. the function returns a variable thisProposalId so the caller can write down what is his proposal number, keep track of it and advertise so other people can sign it!

uint myVote = variablesManager.balanceOf(msg.sender) gets the balance in tokens of the creator of this proposal and stores it in a variable myVote, which will be used later to add to his votes and to the total.

changeProposal memory transferChange; creates an object for the proposal structure. Right now the object is empty, but on the next bits of code we will fill with the data according to the caller data.

uint _currentId=currentId; saves the current ID, very important so we can change the ID but still have this data to return to the user in the end!
currentId++; increases the ID so the next proposal comes after this one.

The following piece of code is just filling the struct of the proposal with the data given to it. The contractId is 1 because this is a transfer proposal!

proposals[_currentId].newAddress=newTransferManager;//address for the new contract
proposals[_currentId].contractID=1;//contract type is transfer(ID=1)
proposals[_currentId].votes[msg.sender]=myVote;//stores I have voted with my tokens
proposals[_currentId].total=myVote;//starting votes is my balance

This next chunk of code checks if the balance of the caller is more than 50%+1 of the total supply of tokens because if it is, it applies the changes right away!

if(myVote>((variablesManager.totalSupply()/2)+1)){//if my balance is more than 50%+1 of supply
            proposals[_currentId].approved=true;//the proposal is approved
            coreManager.setTransferManager(newTransferManager);//new transfer is updated on the core
            variablesManager.setTransferManager(newTransferManager);//new transfer is updated on variable manager
        }

But if it isn't, it sets the proposal as not (yet) accepted:

 else{
            proposals[_currentId].approved=false; //if I have less than enough, proposal is not yet approved
}

And finally, we return the ID so the caller can track his proposal and advertise it:

 thisProposalId=_currentId; //returns the ID of the proposal

Monitoring the status of the change

Again, this is a complex function, so I will show you it in full first, and then explain bit by bit. I consider it important that you understand everything that happens because an understanding of the code will be important for us to fix the previously discussed exploit that allows the voter to be greater than the supply.

    function getChangeProposal(uint proposalId) public view returns(address newAddressProposed, string memory contractType, uint votesFor, bool isApproved){
        changeProposal memory _changeProposal = proposals[proposalId];//stores in memory the object of the proposal
        string memory contractId; //string for telling the contract type of the proposal
        if (_changeProposal.contractID==1){//if id is 1 (transfer)
            contractId="transfer";//remembers it is a transfer contract
        } else if(_changeProposal.contractID==2){//if id is 2(variable)
            contractId="variable";//remembers it is a variable contract
        }
        newAddressProposed=_changeProposal.newAddress; //returns the proposed(new) contract address
        contractType=contractId;//returns the name of the type of the contract (transfer or variable)
        votesFor=_changeProposal.total; //returns total of votes for this proposal
        isApproved=_changeProposal.approved; //returns the status (approved or not)
    }

function getChangeProposal(uint proposalId) The function here receives the Id of the proposal we want to check against the mapping of Id to proposals. On the same line:

returns(address newAddressProposed, string memory contractType, uint votesFor, bool isApproved) We are returning to the user the address, type, votes, and status of the proposal. Everything is the expected data type, except the contractType, here, for a better understanding for the caller, we will return a string, transfer for type 1 and variable for type 2.

changeProposal memory _changeProposal = proposals[proposalId]; we are saving the object in memory to reduce gas costs and improve efficiency as it is better than checking the storage every time.

string memory contractId; is the string we will fill on the decision code below, according to the contract type!

Here, if the contract id is 1, we record on the string above the name "transfer" for better reading for the caller:

if (_changeProposal.contractID==1){//if id is 1 (transfer)
            contractId="transfer";//remembers it is a transfer contract
        }

Below is the same thing, but if it is 2, we record it is a "variable" manager:

} else if(_changeProposal.contractID==2){//if id is 2(variable)
            contractId="variable";//remembers it is a variable contract
        }

Now we just need to return everything we have "discovered" from the proposal:

newAddressProposed=_changeProposal.newAddress; //returns the proposed(new) contract address
contractType=contractId;//returns the name of the type of the contract (transfer or variable)
votesFor=_changeProposal.total; //returns total of votes for this proposal
isApproved=_changeProposal.approved; //returns the status (approved or not)

Voting on a proposal

Time to vote, again, there is an obvious exploit here, discussed on the sections above, but we will ignore because it would require a full tutorial to implement security against this, but on the final version, on another tutorial, this will be fixed.

    function approveChange(uint proposalId) public{
        proposals[proposalId].votes[msg.sender]=variablesManager.balanceOf(msg.sender);//records my vote value
        proposals[proposalId].total+=variablesManager.balanceOf(msg.sender);//adds my votes to the total
        if(proposals[proposalId].total>((variablesManager.totalSupply()/2)+1)){//is total is more than 50%+1
            proposals[proposalId].approved=true;//records the change is approved
            coreManager.setTransferManager(proposals[proposalId].newAddress); //changes the manager on the core
            variablesManager.setTransferManager(proposals[proposalId].newAddress);//changes the manager on the variable manager
        }
    }

The function above receives the Id of the proposal so it can fetch and sign the correct object from the mapping.

The piece of code below is responsible for recording the user intention of voting to the proposal, on the proposal object from the mapping:

        proposals[proposalId].votes[msg.sender]=variablesManager.balanceOf(msg.sender);//records my vote value
        proposals[proposalId].total+=variablesManager.balanceOf(msg.sender);//adds my votes to the total

The last part is checking if the amount of votes on the given proposal is bigger than 50%+1 of the total supply of tokens (again, ignoring the obvious exploit as the fix will require a deeper refactor, that will be done in a standalone tutorial):

if(proposals[proposalId].total>((variablesManager.totalSupply()/2)+1))

In case the above is true, we record that the proposal was accepted with proposals[proposalId].approved=true;.

And then call the function on the core and the variable manager to change the transfer contract! (we will implement in the following sections the core and variable manager part of the code):

coreManager.setTransferManager(proposals[proposalId].newAddress); //changes the manager on the core
variablesManager.setTransferManager(proposals[proposalId].newAddress);//changes the manager on the variable manager

The core and variable manager

Now, we need to prepare the core variable manager to:

• Allow the creator to set the other contracts only and only once.
• After they are set, the creator can't change them by himself, every change will have to be made by the change manager with the voting of the majority of people.
  Notice that the changes are identical both on the core and on the variable manager, so I won't show both two to avoid repetition, but both must be changed.

First, find the transfer manager variable we have set on the previous tutorial, after it, add a boolean variable to keep track if the creator has changed it already or not:

    ERC223Transfers private transferManager;
    bool private transferManagerSet;//tracks if transfer manager is set

Now, apply the same logic for the change manager. But we only need to record the address, not the object:

    address private changeManagerAddress;//address of change manager
    bool private chageManagerSet; //tracks if change manager is set

Now, find the function that sets the transaction manager. You need to change the requirements so that the creator can only set it once. And so that the change manager can change it (the logic for allowing changes are implemented in the change manager):

        //requires that caller is creator or change manager
        //if caller is creator also requires transfer manager was not yet set
        require((msg.sender==_creator&&(!transferManagerSet))||msg.sender==changeManagerAddress);

so either:
msg.sender==_creator&&(!transferManagerSet))
or:
msg.sender==changeManagerAddress

Very similar to the function above, but implemented from scratch, we will allow the creator to set the change manager:

    function setChangeManager(address _changeManager) public{
        require(msg.sender==_creator&&(!chageManagerSet));//requires caller is creator and manager was not yet set
        chageManagerSet=true;//records manager was set
        changeManagerAddress=_changeManager;//records manager address
    }

Done

Now, the change manager is responsible for keeping track of change proposals and making the changes on the other contracts. The other contracts are responsible to allow only the change manager to change the function on them.

Now we will proceed with the testing.

Testing

The testing here is to see if we can change the transfer manager using the change manager. Notice that you must pay attention to the order at which you do things.

First, very similarly to the previous class, I will deploy everything and connect one contract to each other. I won't go over the steps again as they are similar to the last tutorial.

We first tell the core and the variables manager who is the change manager.

I have already constructed the variables on the variable manager like on the previous tutorial.

All the token balance is with the creator 0xca35b7d915458ef540ade6068dfe2f44e8fa733c.
So if the creator proposes a change, it is accepted immediately.

If the address 0x14723a09acff6d2a60dcdf7aa4aff308fddc160c proposes a change it has zero votes.

But if I switch to the creator and vote on it, it is finally approved.

Exploit

During the tests, I have committed a mistake. On the previous classes, I have changed the transaction manager so that the balance is sent to a "hacker" address instead of the real receiver, and I forgot to change it back. So, now the creator has 49 tokens, I have sent 51 to the second address mentioned on the previous section (the one whose proposal was not approved yet), so, though the total supply is 49, 51 tokens are on a wallet that I have lost.

THIS IS THE PERFECT SCENARIO TO EXPLAIN THE EXPLOIT!

I was going to send, anyways, 30 tokens to a third address, and using that address I would open a change proposal, transfer the tokens to another address and vote again, so the tokens would have voted twice.

The contracts have already been set, so the creator can't change them anymore, and I need to fix the hacker exploits from the last class by submitting a change proposal, but I, the creator, and anyone else do not have enough tokens to accept a proposal. But I can push a proposal using the exploit we haven't (yet) fixed. This is interesting because, at the same way I am using a real-world example of how mutable contracts can be useful, again, I am showing that, just like on the last version, at the same time you close an exploit, you open another. It sounds like a zero-sum game!

How it works

The "hacker" from the previous class ha, luckily, only changed the transfer function, without from and without data, so to fix it, I will use the transferFrom without data to transfer from the owner to a wallet I am going to allow, so the receiver receives the balance and avoid the hacked piece of code.

The third address is 0x583031d1113ad414f02576bd6afabfb302140225.

I have approved him to spend 30 tokens from the creators' account.

Now, using the creator, I will submit a proposal of change to a contract without the exploit from the "hacker" from last class.

The proposal has 49 votes already.

I just need to transfer 3 tokens to the new address and vote from there, but using the function that is not hacked and approve from there.

Note: I have noticed at the end of this tutorial only, that there is another exploit I haven't talked about, and this one is even more silly than the previous. One person can vote twice without transferring their tokens! The fix for it is closely related to the fix of the previous exploit. On the next tutorial I will finish the implementation with allowing the change manager to change the variable manager and after that address the exploits.

Curriculum

First tutorial

• (Part 1) Ethereum Solidity Development - Getting Started + Lower Level Explanation (PT 1)

Latest tutorial

• (Part 19) Ethereum Solidity - ERC223 Mutability, Mutable Contract Implementation And Hacking Mutable Contracts(PT 19)

Beneficiaries

This post has as beneficiaries
@utopian.pay with 5%
using the SteemPeak beneficiary tool