search

compound-finance / compound-protocol

The Compound On-Chain Protocol
https://compound.finance/developers
BSD 3-Clause "New" or "Revised" License
1.89k stars 1.21k forks source link

Avoid token reentry attacks #153

Closed wenzhenxiang closed 3 years ago

wenzhenxiang commented 3 years ago

Avoid token reentry attacks, like ERC777 token.

coburncoburn commented 3 years ago

this change should be applied to the redeem function as well

wenzhenxiang commented 3 years ago

this change should be applied to the redeem function as well

yes, the logic of borrow, repay, deposit, redeem needs to be changed to

  1. Validation
  2. UpdateState
  3. Transfer
  4. emit event
coburncoburn commented 3 years ago

this change should be applied to the redeem function as well

yes, the logic of borrow, repay, deposit, redeem needs to be changed to

  1. Validation
  2. UpdateState
  3. Transfer
  4. emit event

There is an additional challenge, since mint and repay need to see the actual amount transferred in order to accurately account for deposits in the case of fee tokens, the transfer must happen before updating state. Since these actions mean the sender is actually in a worse position from internal accounting standpoint before accounting updates, it is okay that they do not follow the pattern.

Borrow and Redeem and Transfer only should be updated

jflatow commented 3 years ago

Now included with #152