code-423n4 / 2023-09-delegate-findings

2 stars 1 forks source link

The owner of the PrincipalToken can redeem the asset from escrow before the selected time period expires #311

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-delegate/blob/main/src/libraries/DelegateTokenStorageHelpers.sol#L162

Vulnerability details

Impact

After calling the create() function, the owner of the DelegateToken gains delegate rights for the duration of the escrow. The documentation for the competition states: "The holder of the PrincipalToken will have the right to redeem the boredom ape from escrow at conclusion of the chosen timeframe." However, this condition is not met in DelegateToken.sol.

Proof of Concept

The user calls the withwraw() function. Function withdraw() (https://github.com/code-423n4/2023-09-delegate/blob/main/src/DelegateToken.sol#L353-L386) - allows principal rights owner or approved operator to withdraw the underlying token once the delegation rights have either met their expiration or been rescinded.

In the withdraw() function, the revertInvalidWithdrawalConditions() function is called, which checks whether the expiry time has expired. However, the check was performed incorrectly: https://github.com/code-423n4/2023-09-delegate/blob/main/src/libraries/DelegateTokenStorageHelpers.sol#L155-L168 if (block.timestamp < readExpiry(delegateTokenInfo, delegateTokenId)). Here it is checked that the expiry time has not yet passed.

Tools Used

Manual review

Recommended Mitigation Steps

Should be if (block.timestamp > readExpiry(delegateTokenInfo, delegateTokenId))

Assessed type

Timing

GalloDaSballo commented 1 year ago

Looks wrong and lacks POC, will flag but unhappy with quality

0xfoobar commented 1 year ago

Believe this is a duplicate of other higher-quality submissions, tough to tell if they even landed on the same bug or not because of the low quality submission

c4-sponsor commented 1 year ago

0xfoobar (sponsor) disputed

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Invalid

GalloDaSballo commented 1 year ago

Closing as low quality