Unused Return from Function Call

Overview

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

What is the Unused Return from Function Call vulnerability?

In Solidity, as in other programming languages, functions may return values as part of their execution. While it is possible that in some circumstances it may be safe to discard these return values, in most cases they should be captured by the caller so as to avoid inadvertent logical errors in program execution. In certain situations, failure to properly evaluate function return values can lead to contract exploits and possible loss of funds.

Further reading: Solidity Documentation: Function Calls

Technical example of vulnerable code

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

  // An external contract that checks user permissions
  contract AccessControl {
      mapping(address => bool) public authorizedUsers;

      // Grant authorization to a user
      function authorizeUser(address _user) external {
          authorizedUsers[_user] = true;
      }

      // Check if the user is authorized
      // Returns `true` if authorized, `false` otherwise
      function isUserAuthorized(address _user) external view returns (bool) {
          return authorizedUsers[_user];
      }
  }

  // A contract that interacts with AccessControl but neglects to check the return value
  contract ResourceAccess {
      AccessControl accessControl;

      // Event to log access attempts
      event AccessAttempt(address user, bool success);

      constructor(address _accessControlAddress) {
          accessControl = AccessControl(_accessControlAddress);
      }

      // Attempts to access a resource without properly checking authorization
      function accessResource() external {
          // Dangerous: The return value of `isUserAuthorized` is ignored
          accessControl.isUserAuthorized(msg.sender);

          // Additional function logic for dealing with the resource goes here.

          // Logic proceeds as if the user is authorized, regardless of the actual authorization status
          emit AccessAttempt(msg.sender, true); // This might inaccurately reflect success
      }
  }

In the example above, there are two separate contracts: one contract, AccessControl, maintains a mapping of authorized users, and exposes a function isUserAuthorized() that can be used to query whether or not a particular address is authorized to use some service. This query is made (incorrectly) in the second contract, ResourceAccess, where the function accessResource() fails to check the return value of isUserAuthorized(), and will allow even unauthorized users to access its functionality. Depending on the nature of this functionality, value in the form of Ether or tokens could be misappropriated by an unauthorized user.

Technical example of how to fix the vulnerability

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

  // An external contract that checks user permissions
  contract AccessControl {
      mapping(address => bool) public authorizedUsers;

      // Grant authorization to a user
      function authorizeUser(address _user) external {
          authorizedUsers[_user] = true;
      }

      // Check if the user is authorized
      // Returns `true` if authorized, `false` otherwise
      function isUserAuthorized(address _user) external view returns (bool) {
          return authorizedUsers[_user];
      }
  }

  contract ResourceAccessCorrected {
      AccessControl accessControl;

      // Event to log access attempts
      event AccessAttempt(address user, bool success);

      constructor(address _accessControlAddress) {
          accessControl = AccessControl(_accessControlAddress);
      }

      // Attempts to access a resource without properly checking authorization
      function accessResource() external {
          // Dangerous: The return value of `isUserAuthorized` is ignored
          bool isAuthorized = accessControl.isUserAuthorized(msg.sender);
          require(isAuthorized, "Caller is not authorized.");

          // Additional function logic for dealing with the resource goes here.

          emit AccessAttempt(msg.sender, true); // This might inaccurately reflect success
      }
  }

In the corrected example, contract ResourceAccessCorrected now captures the return value from isUserAuthorized() and requires that it be true in order to proceed with the body of accessResource().