Shadowing Builtin Name

Overview

• Severity: Low
• Confidence: Medium
• Affected Versions: All

What is the Shadowing Builtin vulnerability?

Solidity provides a number of special variables and functions as part of the language which can be used in smart contracts. In general, the compiler will not prevent the developer from shadowing these names with their own declarations; however, in doing so, they may inadvertently cause errors in their smart contract if they expect to use a builtin symbol that has been shadowed.

Further reading: Solidity Documentation: Special Variables and Functions

Technical example of vulnerable code

  // SPDX-License-Identifier: Unlicense
  pragma solidity ^0.8.0;

  contract MessageHandler {
      // Custom struct to hold in-protocol messages
      struct Message {
          address sender;
          address to;
          string content;
      }

      // Store messages handled by the contract
      mapping(uint => Message) public allMessages;
      uint public messageCount;

      // Also cache latest messages created by each user
      mapping(address => Message) public lastCreatedBy;

      // Function to create a new message
      function createMessage(address _from, address _to, string memory _content) public {
          Message memory msg = Message({
              sender: _from,
              to: _to,
              content: _content
          });

          // Developer intends to access the global 'msg.sender' but accidentally shadows it with 'msg' struct
          lastCreatedBy[msg.sender] = msg;

          allMessages[messageCount] = msg;
          messageCount++;
      }

      // Function to retrieve the sender of the last message
      function getLastMessageSender() public view returns (address) {
          require(messageCount > 0, "No messages exist.");
          return allMessages[messageCount - 1].sender;
      }

      // Additional functionality omitted for brevity
  }

In the example above, contract MessageHandler implements some basic logic for handling and storing custom messages; depending on the content of the messages these could potentially be related to some kind of decentralized finance (DeFi) protocol, though for demonstration purposes they are left generic. In the function createMessage(), a new instance of a custom Message struct is created with the name msg, which is a builtin name in Solidity. When the developer subsequently attempts to access the msg.sender property of the builtin variable, they will instead access the value stored in the struct, which could be different and might lead to subtle logical errors in a protocol using these messages.

Technical example of how to fix the vulnerability

  // SPDX-License-Identifier: Unlicense
  pragma solidity ^0.8.0;

  contract MessageHandlerUpdated {
      // Custom struct to hold in-protocol messages
      struct Message {
          address sender;
          address to;
          string content;
      }

      // Store messages handled by the contract
      mapping(uint => Message) public allMessages;
      uint public messageCount;

      // Also cache latest messages created by each user
      mapping(address => Message) public lastCreatedBy;

      // Function to create a new message
      function createMessage(address _from, address _to, string memory _content) public {
          Message memory newMsg = Message({
              sender: _from,
              to: _to,
              content: _content
          });

          // The msg symbol is no longer shadowed, so this works as intended
          lastCreatedBy[msg.sender] = newMsg;

          allMessages[messageCount] = newMsg;
          messageCount++;
      }

      // Function to retrieve the sender of the last message
      function getLastMessageSender() public view returns (address) {
          require(messageCount > 0, "No messages exist.");
          return allMessages[messageCount - 1].sender;
      }

      // Additional functionality omitted for brevity
  }

In the revised example, contract MessageHandlerUpdated now uses a local struct variable named newMsg, avoiding the shadowing of the builtin that they wish to access for updating the lastCreatedBy mapping.