sherlock-audit / 2022-10-illuminate-judging

3 stars 0 forks source link

__141345__ - Allowance update should use the share, not the amount #197

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

141345

medium

Allowance update should use the share, not the amount

Summary

In withdraw(), the post maturity allowance update deducts the amount of underlying from the old allowance, but PT shares should be used.

Vulnerability Detail

The units for PT is the shares, rather than the underlying token amount. Mix with the two could introduce ambiguity and potential wrong allowance.

Impact

Potentially, this could results in inaccurate amount of allowance being recorded, and in the future lead to users' fund loss.

Code Snippet

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/tokens/ERC5095.sol#L262-L266

Tool used

Manual Review

Recommendation

Use shares instead of a in line 266.

JTraversa commented 1 year ago

While it may be most tautologically accurate to utilize shares instead of a, the use of either can be interchangable in the suggested block of code where users are redeeming post maturity.

141345 commented 1 year ago

Escalate for 21 USDC

This is a duplicate of https://github.com/sherlock-audit/2022-10-illuminate-judging/issues/118

sherlock-admin commented 1 year ago

Escalate for 21 USDC

This is a duplicate of https://github.com/sherlock-audit/2022-10-illuminate-judging/issues/118

You've created a valid escalation for 21 USDC!

To remove the escalation from consideration: Delete your comment. To change the amount you've staked on this escalation: Edit your comment (do not create a new comment).

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.