fibonacci - The functions `VestingEscrow::vote` and `VestingEscrow::voteWithReason` do not return results

[sherlock-admin2]

fibonacci

medium

The functions VestingEscrow::vote and VestingEscrow::voteWithReason do not return results

Summary

These functions are expected to return a result, but the adapter functions to which they delegate the call do not return any values. As a result, they will always return 0 bytes.

Vulnerability Detail

The functions VestingEscrow::vote and VestingEscrow::voteWithReason are expected to return a result of type bytes memory. They delegate the call to the adapter.

function vote(bytes calldata params) external onlyRecipient whenVotingAdaptorIsSet returns (bytes memory) {
    return _votingAdaptor().functionDelegateCall(abi.encodeCall(IVotingAdaptor.vote, (params)));
}

function voteWithReason(bytes calldata params) external onlyRecipient whenVotingAdaptorIsSet returns (bytes memory) {
    return _votingAdaptor().functionDelegateCall(abi.encodeCall(IVotingAdaptor.voteWithReason, (params)));
}

The adapter then interacts with the IGovernor contract, but does not return a result.

function delegate(bytes calldata params) external {
    IVotes(votingToken).delegate(abi.decode(params, (address)));
}

function vote(bytes calldata params) external {
    (uint256 proposalId, uint8 support) = abi.decode(params, (uint256, uint8));
    IGovernor(governor).castVote(proposalId, support);
}

According to the IGovernor interface, the functions castVote and castVoteWithReason return the balance value of type uint256.

function castVote(uint256 proposalId, uint8 support) public virtual returns (uint256 balance);

function castVoteWithReason(
    uint256 proposalId,
    uint8 support,
    string calldata reason
) public virtual returns (uint256 balance);

POC

diff --git a/rio-vesting-escrow/test/VestingEscrow.t.sol b/rio-vesting-escrow/test/VestingEscrow.t.sol
index eafb7dc..9d4c425 100644
--- a/rio-vesting-escrow/test/VestingEscrow.t.sol
+++ b/rio-vesting-escrow/test/VestingEscrow.t.sol
@@ -217,7 +217,8 @@ contract VestingEscrowTest is TestUtil {
         vm.roll(block.number + 1);

         vm.prank(recipient);
-        deployedVesting.vote(ozVotingAdaptor.encodeVoteCallData(proposalId, uint8(VoteType.For)));
+        bytes memory result = deployedVesting.vote(ozVotingAdaptor.encodeVoteCallData(proposalId, uint8(VoteType.For)));
+        assertEq(result.length, 0);

         (uint256 againstVotes, uint256 forVotes, uint256 abstainVotes) = governor.proposalVotes(proposalId);

Impact

This unexpected behavior could lead to errors when interacting with the protocol.

Code Snippet

https://github.com/sherlock-audit/2024-01-rio-vesting-escrow/blob/main/rio-vesting-escrow/src/VestingEscrow.sol#L152-L163 https://github.com/sherlock-audit/2024-01-rio-vesting-escrow/blob/main/rio-vesting-escrow/src/adaptors/OZVotingAdaptor.sol#L61-L73

Tool used

Manual Review

Recommendation

diff --git a/rio-vesting-escrow/src/adaptors/OZVotingAdaptor.sol b/rio-vesting-escrow/src/adaptors/OZVotingAdaptor.sol
index c9c8556..39616bb 100644
--- a/rio-vesting-escrow/src/adaptors/OZVotingAdaptor.sol
+++ b/rio-vesting-escrow/src/adaptors/OZVotingAdaptor.sol
@@ -60,16 +60,16 @@ contract OZVotingAdaptor is IVotingAdaptor, Ownable {

     /// @notice Vote on an OZ proposal.
     /// @param params The ABI-encoded proposal id and support value.
-    function vote(bytes calldata params) external {
+    function vote(bytes calldata params) external returns (uint256 balance) {
         (uint256 proposalId, uint8 support) = abi.decode(params, (uint256, uint8));
-        IGovernor(governor).castVote(proposalId, support);
+        return IGovernor(governor).castVote(proposalId, support);
     }

     /// @notice Vote on a proposal with a reason.
     /// @param params The ABI-encoded proposal id, support value, and reason.
-    function voteWithReason(bytes calldata params) external {
+    function voteWithReason(bytes calldata params) external returns (uint256 balance) {
         (uint256 proposalId, uint8 support, string memory reason) = abi.decode(params, (uint256, uint8, string));
-        IGovernor(governor).castVoteWithReason(proposalId, support, reason);
+        return IGovernor(governor).castVoteWithReason(proposalId, support, reason);
     }

     /// @notice Recover any ERC20 to the owner.
diff --git a/rio-vesting-escrow/src/interfaces/IVotingAdaptor.sol b/rio-vesting-escrow/src/interfaces/IVotingAdaptor.sol
index c5c9c02..7250183 100644
--- a/rio-vesting-escrow/src/interfaces/IVotingAdaptor.sol
+++ b/rio-vesting-escrow/src/interfaces/IVotingAdaptor.sol
@@ -23,9 +23,9 @@ interface IVotingAdaptor {

     /// @notice Vote on a proposal.
     /// @param params The ABI-encoded vote params.
-    function vote(bytes calldata params) external;
+    function vote(bytes calldata params) external returns (uint256 balance);

     /// @notice Vote on a proposal with a reason.
     /// @param params The ABI-encoded vote with reason params.
-    function voteWithReason(bytes calldata params) external;
+    function voteWithReason(bytes calldata params) external returns (uint256 balance);
 }

Duplicate of #80

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

_rahul commented:

Invalid. Does not cause tangible loss of funds.