Skip links

Security Auditing: Watch for Duplicated Storage in Solidity Smart Contracts

Solidity semantics are confusing for smart contract developers with experience in traditional programming languages. This semantics can lead to security issues like the one we found in a recent smart contract security audit we did. The following code caught our attention:

contract Storage {
  struct Info {
    address owner;
    uint amount;
  }
  mapping (address => Info) addressInfo;
  mapping (uint => Info) indexInfo;
  uint index;
  function create(uint _amount) {
    var info = Info(msg.sender, _amount);
    indexInfo[index] = info;
    addressInfo[msg.sender] = info;
    ++index;
  }
  function burn(address _to, uint _amount) {
    var info = addressInfo[msg.sender];
    info.amount = safeSub(info.amount, _amount);
  }
}

In the above code, the create method stores the same information in two different mappings, not a reference. However, when we assign a struct, we get a reference which makes Solidity ambiguous.

A quick test confirms that the compiler is creating two copies. Initially, we thought it was just wasteful to store two copies because it is more expensive. We could save some gas by using an indirect index:

mapping (address => Info) addressInfo;
mapping (uint => address) indirectIndex;
function create(uint _amount) {
  var info = Info(msg.sender, _amount);
  addressInfo[msg.sender] = info;
  indexInfo[index] = msg.sender;
  ++index;
}

The indirect index also solves a more serious problem that the copies created in the burn method: if the copies are not synchronized, they will eventually contain different information:

function burn(address _to, uint _amount) {
  var info = addressInfo[msg.sender];
  info.amount = safeSub(info.amount, _amount);
}

If they had an indirect index they will not have such problem.

Conclusion

Solidity mappings are an essential part of most smart contracts, so it helps to get used to the way they work. Every Solidity developer should be aware of the quirks of Solidity mappings. Please see Solidity Frustrations: References and Mapping for examples.
So, to recap, it is important to avoid storing the same information in two mappings.

If you liked this article, you might also like: