code-423n4 / 2023-01-drips-findings

0 stars 2 forks source link

callSigned() does not check the relationship between the caller and the sender #188

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-drips/blob/main/src/Caller.sol#L164-L183

Vulnerability details

Impact

The function of the callsigned() function is Makes a call on behalf of the sender. But it does not check the direct relationship between msg.sender and the sender, and the attacker can preempt the user to call.

function callSigned(
  address sender,
  address to,
  bytes memory data,
  uint256 deadline,
  bytes32 r,
  bytes32 sv
) public payable returns (bytes memory returnData) {
  // slither-disable-next-line timestamp
  require(block.timestamp <= deadline, "Execution deadline expired");
  uint256 currNonce = nonce[sender]++;
  bytes32 executeHash = keccak256(
      abi.encode(
          callSignedTypeHash, sender, to, keccak256(data), msg.value, currNonce, deadline
      )
  );
  address signer = ECDSA.recover(_hashTypedDataV4(executeHash), r, sv);
  require(signer == sender, "Invalid signature");//@audit  
  return _call(sender, to, data, msg.value);
}

Proof of Concept

function callSigned(
  address sender,
  address to,
  bytes memory data,
  uint256 deadline,
  bytes32 r,
  bytes32 sv
) public payable returns (bytes memory returnData) {
  // slither-disable-next-line timestamp
  require(block.timestamp <= deadline, "Execution deadline expired");
  uint256 currNonce = nonce[sender]++;
  bytes32 executeHash = keccak256(
      abi.encode(
          callSignedTypeHash, sender, to, keccak256(data), msg.value, currNonce, deadline
      )
  );
  address signer = ECDSA.recover(_hashTypedDataV4(executeHash), r, sv);
  require(signer == sender, "Invalid signature");//@audit  
  return _call(sender, to, data, msg.value);
}

Utilization scenario: When a user calls callsigned(), but suddenly wants to cancel it. But an attacker can monitor this operation. Executed successfully for the user

Tools Used

vscode

Recommended Mitigation Steps

Check the relationship between msg.sender and sender

GalloDaSballo commented 1 year ago

Anyone can perform the call, they are authorized because the signature allows them to

If they send value, they are paying on the signer == sender behalf

This looks off

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Insufficient proof