Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flashloan Detector 02.02 - Update #17

Merged
merged 36 commits into from
Dec 21, 2022
Merged

Flashloan Detector 02.02 - Update #17

merged 36 commits into from
Dec 21, 2022

Conversation

RCantu92
Copy link

@RCantu92 RCantu92 commented Dec 14, 2022

  • Update to track the profit of the end recipient instead of the transaction initiator via looping through transaction traces in reverse
  • Update bot tests to check for the destination address of a ERC20 Transfer event and native coin transfers
  • Add new test for case when end recipient is not the transaction initiator
  • Update tests to properly utilize traces
  • Add labels property to findings
  • Add coverage for Balancer and DODO flashloans

@RCantu92 RCantu92 marked this pull request as ready for review December 16, 2022 06:30
@RCantu92 RCantu92 requested a review from Vxatz December 16, 2022 06:31
@Vxatz
Copy link

Vxatz commented Dec 16, 2022

Trying to test the bot with this tx that we discussed, I get a false negative. That seems to be because, while the native profits are calculated correctly, when iterating through the back of Transfers, that last event that enters the for-loop and eventually breaks out of it, is the Log#5 from Etherscan which is an intermediate transfer from the "attacker"'s contract to Bancor: Converter.

It specifically enters the else if (name === "Transfer" && src.toLowerCase() === calledContract)which results in calculating the balance change of the Bancor: Convert in the const tokenProfits = helper.calculateTokenProfits(transferEvents, dst.toLowerCase());

I think that's happening because the last thing happening in the transaction (which results in the profit of the attacker) is the ETH transfer in the end.

A possible mitigation is to check if the last thing that happened in the transaction is an ERC20 or an ETH transfer. If it's the latter, then the bot shouldn't check the end recipient from the ERC20 transfer events. To do this, the bot could, for every txEvent, loop through the traces and check which of the following comes "later": a) a trace with trace.action.value, or b) a trace with trace.action.input.startsWith(transferSig) || trace.action.input.startsWith(transferFromSig).

@Vxatz
Copy link

Vxatz commented Dec 16, 2022

The Balancer flashloan signature can be found in the transaction that was given as an example in the recently opened issue on this repo (tx link)

This seems a simple fix, basically creating a balancer-detector.jswith what is contained in the maker-detector.js, except changing the address tokento address indexed token (and then adding it to flashloan-detector.js)

On the same topic, there's an issue opened on our main repo, for which, while I think we don't need to change the Pancake's Large-Swap bot, we could also include that flashloan's signature to this bot as well.
The tx that the issue is referring to is this and the flashloan event can be found by searching for "DODOFlashLoan" in the Logs tab. This is a bit trickier, because in order to find the token that was flashloaned, we need to call _BASE_TOKEN_or/and _QUOTE_TOKEN_ on the address that emitted the event (DODO pool) for the non-zero amounts baseAmount/quoteAmountof the event.

@RCantu92 RCantu92 self-assigned this Dec 20, 2022
@RCantu92 RCantu92 requested a review from Vxatz December 20, 2022 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants