code-423n4 / 2023-07-pooltogether-findings

12 stars 7 forks source link

Anyone could call `sponsor()` which is not in line with desired behaviour #285

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/GenerationSoftware/pt-v5-twab-controller/blob/0145eeac23301ee5338c659422dd6d69234f5d50/src/TwabController.sol#L493-L502 https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L474-L482

Vulnerability details

Impact

Break in contract's logic since a different entity other than the Vault contract makes this execution

Proof of Concept

Take a look at TwabController.sol#L493-L502

  }

  /**
   * @notice Delegate user balance to the sponsorship address.
   * @dev Must only be called by the Vault contract.
   * @param _from Address of the user delegating their balance to the sponsorship address.
   */
  function sponsor(address _from) external {
    _delegate(msg.sender, _from, SPONSORSHIP_ADDRESS);
  }

As clearly stated by the comment at L497 this function should only be called by the vault contract, but there is actually no validation in the code blocks to ensure that this is always the case.

Note that where as the function can be called from the Vault.sol contract as expected but there should also exist a check to confirm in the twab controller to ensure that the execution is being queried by the Vault contract.

Tool used

Manual Audit

Recommended Mitigation Steps

Introduce necessary validation measures or instead correct the documentation that any one could execute this function in the stead of the vault contract

Assessed type

Invalid Validation

Picodes commented 1 year ago

No impact is described

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Invalid