sherlock-audit / 2024-04-interest-rate-model-judging

9 stars 5 forks source link

ether_sky - Missing nonce in the permitSender modifier. #168

Closed sherlock-admin4 closed 4 months ago

sherlock-admin4 commented 5 months ago

ether_sky

medium

Missing nonce in the permitSender modifier.

Summary

Vulnerability Detail

The nonce is missing here.

modifier permitSender(ClaimPermit calldata permit) {
  assert(_claimSender == address(0));
  assert(permit.deadline >= block.timestamp);
  unchecked {
    address recoveredAddress = ecrecover(
      keccak256(
        abi.encodePacked(
          "\x19\x01",
          DOMAIN_SEPARATOR(),
          keccak256(
            abi.encode(
                keccak256("ClaimPermit(address owner,address spender,address[] assets,uint256 deadline)"), // @audit, here
              permit.owner,
              msg.sender,
              permit.assets,
              nonces[permit.owner]++,
              permit.deadline
            )
          )
        )
      ),
      permit.v,
      permit.r,
      permit.s
    );
    assert(recoveredAddress != address(0) && recoveredAddress == permit.owner);
    _claimSender = permit.owner;
  }
  _;
  assert(_claimSender == address(0));
}

Impact

Code Snippet

https://github.com/sherlock-audit/2024-04-interest-rate-model/blob/8f6ef1b0868d3ea3a98a5ab7e8b3a164857681d7/protocol/contracts/RewardsController.sol#L754

Tool used

Manual Review

Recommendation

modifier permitSender(ClaimPermit calldata permit) {
  assert(_claimSender == address(0));
  assert(permit.deadline >= block.timestamp);
  unchecked {
    address recoveredAddress = ecrecover(
      keccak256(
        abi.encodePacked(
          "\x19\x01",
          DOMAIN_SEPARATOR(),
          keccak256(
            abi.encode(
-                keccak256("ClaimPermit(address owner,address spender,address[] assets,uint256 deadline)"),
+                keccak256("ClaimPermit(address owner,address spender,address[] assets,uint256 nonce,uint256 deadline)"),
              permit.owner,
              msg.sender,
              permit.assets,
              nonces[permit.owner]++,
              permit.deadline
            )
          )
        )
      ),
      permit.v,
      permit.r,
      permit.s
    );
    assert(recoveredAddress != address(0) && recoveredAddress == permit.owner);
    _claimSender = permit.owner;
  }
  _;
  assert(_claimSender == address(0));
}
santipu03 commented 4 months ago

This issue is invalid because it doesn't have any impact. It's not possible to replay a signature because there's an application-level nonce used in the signature:

nonces[permit.owner]++