Array Parameter Location

Overview

• Severity: High
• Confidence: High
• Affected Versions: < 0.5.0

What is the Array Parameter Location vulnerability?

In Ethereum, arrays may be located in storage (as part of the stored state associated with a smart contract), or memory (ephemeral data that exist only during a smart contract's function execution). In earlier versions of Solidity, arrays passed as function parameters could be specified without a location keyword, in which case they would default to memory. This could lead to unexpected behavior if the function was intended to modify the state of the smart contract.

Further reading: Solidity Documentation: Data Location

Technical example of vulnerable code

  // SPDX-License-Identifier: Unlicense
  pragma solidity 0.4.0;

  contract AmbiguousArrayLocation {
      address public owner;
      address[3] public contractAdmins;
      uint8 nextUnusedAdminSlot = 0;

      constructor() {
          owner = msg.sender;
      }

      function addAdmin(address _address) public {
          require(owner == msg.sender);
          require(nextUnusedAdminSlot < 3);
          updateAdmin(contractAdmins, nextUnusedAdminSlot, _address);
          nextUnusedAdminSlot = nextUnusedAdminSlot + 1;
      }

      function removeAdminBySlot(uint8 _adminSlot) public {
          require(owner == msg.sender);
          require(_adminSlot < 3);
          clearArraySlot(contractAdmins, _adminSlot);
      }

      function updateAdmin(address[3] storage _admins,
                           uint8 _adminId,
                           address _newAdminAddress) internal {
          _admins[_adminId] = _newAdminAddress;
      }

      function clearArraySlot(address[3] _admins, uint8 _slot) internal {
        _admins[_slot_] = address(0);
      }

      // additional smart contract functionality

  }

In the example above, the AmbiguousArrayLocation contract allows an owner to specify up to three contract administrators through the contractAdmins array, which is in storage. The addAdmin function allows the owner to add an administrator to the array, and the removeAdminBySlot function allows the owner to remove an administrator by specifying the index of the array to clear. The updateAdmin function is used internally to update the array, and the clearArraySlot function is used internally to clear an array slot.

The potential vulnerability with this code lies in the declaration of the clearArraySlot() function. While the updateAdmin() function correctly declares the _admins array parameter to be a storage array, the clearArraySlot() function does not include a location keyword, meaning in this case that it will default to memory. As a result, the admin address will not actually be removed in the state of the contract, and any functions that are restricted to admins only in the contract will still allow execution by an admin who should have had that privilege removed.

Technical example of how to fix the vulnerability

  // SPDX-License-Identifier: Unlicense
  pragma solidity 0.4.0;

  contract StorageArrayLocation {
      address public owner;
      address[3] public contractAdmins;
      uint8 nextUnusedAdminSlot = 0;

      constructor() {
          owner = msg.sender;
      }

      function addAdmin(address _address) public {
          require(owner == msg.sender);
          require(nextUnusedAdminSlot < 3);
          updateAdmin(contractAdmins, nextUnusedAdminSlot, _address);
          nextUnusedAdminSlot = nextUnusedAdminSlot + 1;
      }

      function removeAdminBySlot(uint8 _adminSlot) public {
          require(owner == msg.sender);
          require(_adminSlot < 3);
          clearArraySlot(contractAdmins, _adminSlot);
      }

      function updateAdmin(address[3] storage _admins,
                           uint8 _adminId,
                           address _newAdminAddress) internal {
          _admins[_adminId] = _newAdminAddress;
      }

      function clearArraySlot(address[3] storage _admins, uint8 _slot) internal {
          _admins[_slot_] = address(0);
      }

      // additional smart contract functionality

  }

In the corrected StorageArrayLocation contract, the array parameter of the clearArraySlot() function has been updated to include the storage keyword, indicating that the contractAdmins array on the contract should be updated to zero out the address at the specified index.