sherlock-audit / 2023-07-blueberry-judging

2 stars 1 forks source link

feelereth - reentrancy vulnerability in the _doERC20TransferIn and _doERC1155TransferIn functions #76

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

feelereth

high

reentrancy vulnerability in the _doERC20TransferIn and _doERC1155TransferIn functions

Summary

There is a reentrancy vulnerability in the _doERC20TransferIn and _doERC1155TransferIn functions. The issue is that these functions first call transferFrom on an external token contract to transfer tokens into the BlueBerryBank contract, and only afterwards update the internal balances.

Vulnerability Detail

The key issue is that these functions call transferFrom on external tokens to transfer tokens into the contract, before updating the internal balances. This means that if the token contract is malicious, it could make a recursive call back into BlueBerryBank to drain funds before the balances are updated. Here is how it could be exploited:

  1. Attacker calls execute() to enter execution mode
  2. Attacker calls _doERC20TransferIn() or _doERC1155TransferIn(), specifying a malicious token contract they control
  3. The function calls transferFrom on the malicious token
  4. The malicious token's transferFrom implementation recursively calls back into BlueBerryBank, e.g. calling takeCollateral()
  5. Since balances weren't updated yet, takeCollateral still sees the old balances and allows draining more tokens
  6. Repeat steps 4-5 recursively until drained

Impact

Attackers can drain funds from BlueBerryBank by recursively calling back in before internal state is updated

Code Snippet

https://github.com/sherlock-audit/2023-07-blueberry/blob/main/blueberry-core/contracts/BlueBerryBank.sol#L944-L947 https://github.com/sherlock-audit/2023-07-blueberry/blob/main/blueberry-core/contracts/BlueBerryBank.sol#L951-L955 https://github.com/sherlock-audit/2023-07-blueberry/blob/main/blueberry-core/contracts/BlueBerryBank.sol#L956-L960 https://github.com/sherlock-audit/2023-07-blueberry/blob/main/blueberry-core/contracts/BlueBerryBank.sol#L976-L980 https://github.com/sherlock-audit/2023-07-blueberry/blob/main/blueberry-core/contracts/BlueBerryBank.sol#L967-L970 https://github.com/sherlock-audit/2023-07-blueberry/blob/main/blueberry-core/contracts/BlueBerryBank.sol#L983-L988

Tool used

Manual Review

Recommendation

update the balances before calling the external transferFrom

sherlock-admin2 commented 1 year ago

4 comment(s) were left on this issue during the judging contest.

shogoki commented:

Tokens are whitelisted

0xyPhilic commented:

invalid because functions that call internal _doTransferIn have re-entrancy protection + protocol works only with whitelisted tokens

darkart commented:

Invalid

Kral01 commented:

cant interact with a mallcious token contract