CORIONplatform / solidity

GNU General Public License v3.0
12 stars 9 forks source link

owned.sol - temporary critical vulnerability after creation #161

Closed gundas closed 6 years ago

gundas commented 6 years ago

A number of contracts (tokenDB, providerDB, schellingLight and exchangeLight) extend the owned contract.

Once such contract is created on the blockchain, the owner of the contract is not set - anyone can call replaceOwner() function and take-over the contract. Or, for example, anyone could create unlimited number of Tokens by calling increase(..) function on the 'tokenDB' while the owner is not set yet.

I would suggest copying the implementation from OpenZeppelin, where the owner is always first set to the creator of the contract:

  function owned() {
    owner = msg.sender;
  }
iFA88 commented 6 years ago

Hey, yes I know this. This is not a vulnerability! If I deploy first time a database contract, I need create with an empty owner because I need pair the two contract. For pairing I need to know every address, but on the first deploy I don't know the second contract address.

The process is:

At the end I have an paired contract prepared for replace.

So it should be.

If you check schellingLight and exchangeLight contract constructor functions, the owner are set :)

gundas commented 6 years ago

I still think it is a big security risk - there is an opportunity to mint an unlimited amount of tokens while the owner is not set.

There is no reason to have this risk:

iFA88 commented 6 years ago

Yeah, this mean after I deploy the database contract the attacker should fast call the contract, before I deploy the second (Main) contract.

  1. I think this will never happen.
  2. If this happens, I drop the contracts and don't use them.

+1 The database contracts is on the blockchain with the correct owner. No one can call that except the owner.