code-423n4 / 2021-10-union-findings

0 stars 0 forks source link

Unneeded Named Returns (UToken.sol) #18

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

ye0lde

Vulnerability details

Impact

Removing named return variables can reduce gas usage and improve code clarity.

Proof of Concept

These named return variables are part of three-line functions and the variable is only used to set the return value. While that's not incorrect it is not needed and inconsistent with the way these three line functions are handled in the rest of the project.

For example, here are two almost identical functions, one using named returns and one not. https://github.com/code-423n4/2021-10-union/blob/4176c366986e6d1a6b3f6ec0079ba547b040ac0f/contracts/market/UToken.sol#L288-L304

Functions using named returns in this way are: https://github.com/code-423n4/2021-10-union/blob/4176c366986e6d1a6b3f6ec0079ba547b040ac0f/contracts/market/UToken.sol#L224-L225 https://github.com/code-423n4/2021-10-union/blob/4176c366986e6d1a6b3f6ec0079ba547b040ac0f/contracts/market/UToken.sol#L233-L234 https://github.com/code-423n4/2021-10-union/blob/4176c366986e6d1a6b3f6ec0079ba547b040ac0f/contracts/market/UToken.sol#L293-L294

Tools Used

Visual Studio Code, Remix

Recommended Mitigation Steps

Remove the named returns.

For example, replace function getBorrowed(address account) public view returns (uint256 borrowed) { borrowed = accountBorrows[account].principal; } with function getBorrowed(address account) public view returns (uint256) { return accountBorrows[account].principal; }

Or if there is a reason to use named returns then change the other similar functions to use them.

GalloDaSballo commented 3 years ago

Agree that there is contradiction here, changing this to non-critical

Would recommend the sponsor to decide to either always use named returns or never use them for the sake of consistency