Open code423n4 opened 2 years ago
Since the function internally deals with shares rather than the underlying tokens the implementation is actually correct. We'll make it more explicit in the event definition. Since the issue deals with off-chain monitoring we suggest a rating of (0) Non-critical
0 — Non-critical: Code style, clarity, syntax, versioning, off-chain monitoring (events etc), exclude gas-optimisations.
Handle
WatchPug
Vulnerability details
For the
Transfer
event,amount
is expected to be the amount of tokens transferred, usually equal to the allowance decreased.However, in the
transfer()
function, at L402:_amount
is transformed into shares.https://github.com/code-423n4/2021-12-sublime/blob/9df1b7c4247f8631647c7627a8da9bdc16db8b11/contracts/SavingsAccount/SavingsAccount.sol#L393-L416
In the
transferFrom()
function, at L441:_amount
is transformed into shares.https://github.com/code-423n4/2021-12-sublime/blob/9df1b7c4247f8631647c7627a8da9bdc16db8b11/contracts/SavingsAccount/SavingsAccount.sol#L426-L456
As a result, the
amount
inTransfer
events is wrong.PoC
Given:
1.2
12,000 USDC
toyearn
strategy, received10,000
share tokens;transfer()
12,000 USDC
to Bob;Transfer
event is:12,000
;Transfer
event is:10,000
.