[Audit Fix] Serialization can be invoked by anyone

[dmvt]

Summary

A sophisticated hacker can trick access checks in model contracts to think that he is a legit sender. He can set arbitrary values to be serialized in the storage.

Risk Rating

4

Vulnerability Details

Model contracts DAO, Ecosystem and Token have the authorization checks in serialization function, however, these checks are not effective as the caller can pass any values inside the nested object. For instance, DAO contract has such an auth check:

require(
    msg.sender == _record.uuid || msg.sender == _record.ecosystem.configuratorAddress,
    'ElasticDAO: Unauthorized'
  );

The second part of the OR condition is useless as it is possible to pass a _record struct with a nested ecosystem struct that is built with your parameters and can trick the contract to think that you are the configurator.

Impact

The impact is huge. The storage cannot be trusted as anyone can overwrite it. Without proper storage, it is impossible to use the ElasticDAO contracts as they heavily rely on this.

Proof of Concept

See the contract that I wrote and attached to this gist: ExploitDaoSerialize.sol

It is a simplified version demonstrating how a DAO model contract can be exploited. Open this file in Remix IDE (you can use something else if you have a preference). Compile and deploy DAO contract, then deploy the Attacker contract, specify the DAO contract address and a target address that can be any address for the testing purposes. Then invoke function deserialize on DOA contract passing the same target address and see that the data has been updated to the values set in the Attacker contract. Other model contracts can be exploited in a similar fashion.

Definition of Done

• Configurator functionality is moved to ElasticDAO.sol
• Configurator address is removed from the guards in the 3 models' respective serialize functions