code-423n4 / 2021-09-yaxis-findings

0 stars 0 forks source link

Carefully add tokens to the list that the protocol uses #161

Closed code423n4 closed 2 years ago

code423n4 commented 3 years ago

Handle

tensors

Vulnerability details

Impact

As of right now I believe the only outside tokens the protocol uses are DAI, USDC, USDT and WETH. If other tokens are added, make sure to check that they have no callbacks on transfer.

For example, CREAM protocol added the AMP token which has a callback before a transfer, resulting in an 18 million dollar hack. Had the dev team carefully vetted tokens that are added to the protocol, the attack would not have happened.

Proof of Concept

Through the code .safeTransfer() is used, which then calls back to the receiver if the token is a callback on transfer token. This is an example where using .transfer() would actually be safer than using .safeTransfer().

Recommended Mitigation Steps

Make sure that developers are aware of this attack vector, an add this to the list of considerations when letting the user deposit other tokens.

BobbyYaxis commented 3 years ago

Operational documentation and governance avoids this, not sure it's labelled as a bug in the code to be fixed.

GalloDaSballo commented 2 years ago

Duplicate of #11