Warning: Solution to the challenge will be discussed here. Also note, I could be totally fucking wrong about everything I’m about to write. This is mostly just me long form “tweeting into the ether” about my smart contract journey. Proceed with caution.

There’s a lending pool with a million DVT tokens in balance, offering flash loans for free.

If only there was a way to attack and stop the pool from offering flash loans …

You start with 100 DVT tokens in balance.

So from jump, this particular problem is pretty clear that we don’t need to necessarily try to drain the contract or anything but just stop it from offering flash loans. I’m not really sure how that’s done without bankrupting the contract, so let’s take a look at the code…

Checking out the Receiver contract for the flash loan and I’m not noticing anything too wonky here…

Here we just set the owner of the contract to match the address that deployed the contract initially as well as set the pool (of type UnstoppableLender) to point to the contract we’re hoping to freeze. Nothing special to see here (or is there?).

This is an external function meant to be called by UnstoppableLender in order to move tokens to the ReceiverUnstoppable , specifying two arguments the tokenAddress which is basically the address of the contract holding the state for a particular ERC20 that we are trying to transfer to Receiver , and an amount that details exactly how many tokens we should be sending.

From the first line of this function, we see that in order for this function to not revert, the address that called this particular function must have come from the pool variable we set up in the constructor.

Sidenote: the transfer method is a method that looks like this for any ERC20, where we can move value from the contract to whichever address we specify _to with an appropriate _value and returns a bool indicating if the call was successful or not.

function transfer(address _to, uint256 _value) public returns (bool success)

After we satisfy the first condition, we move onto the second requirement that casts the ERC20 standard over the provided tokenAddress in the arguments (to ensure we have access to that transfer f(x) you see up there).

So this seems to be pretty on par with a flash loan recipients’ obligations to their lender. As long as we’re getting tokens from the DVT pool and then sending them back at the end all of the state changes we attempt with this flash loan should be accepted.

So this looks like a f(x) that’s meant to be called by the owner of the ReceiverUnstoppable contract that was set in the constructor and it only expects an amount to request in the form of a flash loan. The final line of this function appears to call on the pool object that’s pointing at the DVT contract to make a request for some ⚡️ loans.

I’m seeing a lot of OpenZeppelin standards at the top which gives me very little faith that I’ll be draining this contract completely. I’m pretty new to this but typically seeing these libraries means I’m not gonna be able to rely on reentrancy or arithmetic bs here. There doesn’t seem to be anything about ownership of contract though… 🧐

I’m also noticing that this contract seems to have an interface for the receiveTokens f(x) that’s written out in our other contract of concern for this challenge.

So a few things, we’re using SafeMath to manage arithmetic and try to stop overflows, so it’s not likely that we’ll be able to fuck off poolBalance which is of type uint256, and then we have public access to the DamnValuableToken ERC20 implementation as well as the poolBalance that seems pretty well guarded although visible.

Here, we just want to provide a token address that should point to the location of the DamnValuableToken ERC20 implementation and make sure it’s not pointing to the null address address(0) at which point we point our DamnValuableToken at the tokenAddress provided by msg.sender when they deployed the function.

Something about the use of a wrapper deposit function instead of using native functions within ERC20 standard feels intuitively off to me, but I’m also pretty ignorant as to what current best practices are for most shit like this right now. But this particular wrapper is expecting an amount to be provided by the address that calls this function and we are doing a check immediately to make sure that msg.sender isn’t calling this with zero tokens to deposit.

Sidenote: the transferFrom method is again part of the ERC20 standard, that differentiates from the previously inspected transfer only in that we are not explicitly deciding which address the value should be sourced from. Notice that in this particular call we need to specify this because we are transferring value between users within the DamnValuableToken contract, so we need to know both parties in advance and cannot implicitly derive this from msg.sender . F(x) returns bool on success but should throw unless the _from account has deliberately authorized the sender of the message via some mechanism

function transferFrom(address _from, address _to, uint256 _value) public returns (bool success)

Then the poolBalance should be incremented to reflect the updated balance after you call this function as the last step.

This is pretty weird because I don’t see any clear fallback function or anything that will manage incrementing poolBalance if we decide to send/transfer funds to UnstoppableLender through ERC20 methods rather than using this depositTokens wrapper. That seems weird but maybe this is a normal thing.

This seems to be the last stop on our journey and I’m seeing a lot of small clues being confirmed about flashLoan from up until this point. We could’ve guessed it was external or public since we were calling it from within our ReceiverUnstoppable contract, and I assumed it would be guarded against reentrancy which turns out to be a good assumption.

And it looks like this function is taking in an amount argument which we could infer from the fact that we supplied an argument by this name from within ReceiverUnstoppable when we make the call to executeFlashLoan

This f(x) is giving me an odd tingly sensation in my brain as I’m scanning it, I’m pretty certain it’s just me feeling excitement that the error must be in here through some sort of process of elimination.

So we have a quick check to make sure that we aren’t trying to request a loan for zilch (although I wonder what kinda attack surface that enables), and then we get a read on how many DVT tokens our current contract holds by checking its state within DVT.

Sidenote: balanceOf is another method that can be found as part of the ERC20 standard and it’s a pretty straightforward public view function that lets us specify an address that we would like to see the quantity of tokens within the contract allocated to them

function balanceOf(address _owner) public view returns (uint256 balance)

So boom, we check out balance by passing in address(this) and make sure that the UnstoppableLender actually has enough tokens to fulfill their part of the obligation for the ⚡️ loan…. and the catch is on line 36.

We are asserting that poolBalance should match the reported state from balanceBefore — it’s really important to note here that balanceBefore is reading state directly from DVT contract while poolBalance is attempting to map directly to that balanceBefore variable. I think there are two ways I can stop the function given this line of code:

1. Use transfer or send methods to send one of our 10 DVT tokens to either the DVT contract specifying that we should increase the balance for UnstoppableLender’s address and so balanceBefore shouldn’t match with poolBalance after we’re done.

2. Same thing as (1) but sending DVT to UnstoppableLender contract directly (without using the depositTokens so that poolBalance doesn’t get incremented by value) and in this case poolBalance should be ahead of balanceBefore

In either of these two cases, our assert should fail and flash loans should no longer be possible.

Going forward, it looks like if the assertion clears then we are going to hit the DVT contract and ask it to transfer over tokens to msg.sender ’s address for the total borrow amount. Once that transfer is made and approved, we then call the receiveTokens interface with the msg.sender as the target to receive the tokens which are returned within the same call.

Once we make exit the IReceiver call then we should have received back full payment for the loan which is verified by the last two lines of the the smart contract that are checking the balance after the flash loan has been provided. If the balance hasn’t been paid back all of the changes associated with these function calls would be reverted and the gas from sender would be lost in the ether :(

The exploit method snippet depicted

So this was all we had to do to solve this one. Just shoot a token to the pool.

This was pretty fun for a first challenge. One thing that struck a personal cord for me with this one is to really reason about your safety checks and what exactly you’re trying to protect especially within functions that are making calls to potentially multiple other contracts/functions. It was kind of daunting just sitting on line 36 for a few minutes to really think about why my brain started freaking out a bit when it saw the line. I can think of a few workarounds for this one that might work, in order of my level of faith in the revision being functional:

1. We change the poolBalance value to instead reflect address(this).balance , so we are always operating with the current total balance in our UnstoppableLender and make sure that it matches with balanceBefore if we absolutely *must *check to be sure that separate states are matching

2. I probably wouldn’t even bother with trying to match a separate state within UnstoppableLender — as far as I can tell, we could’ve just checked to be sure balanceBefore wasn’t 0 and proceeded from there, but I could also be missing something in my analysis as to why we need to have the poolBalance mapping to DVT balance

3. Write a fallback function that updates poolBalance anytime money is deposited into the contract. Something in my gut tells me this is bad, but I’ll have to experiment later

Thanks for sifting through this if you made it to the end. I don’t normally write out stuff like this, but I’m trying a new thing; and I would love to hear more feedback on how I can make this sort of shit more accessible and useful to others or just general improvements/critiques/comments/whatever. \n \n This is literally just a brain dump as I’m trying to learn more about this stuff and I really really appreciate any feedback or insights into what I got wrong or what I should revisit my mental model for!

Warning: Solution to the challenge will be discussed here. Also note, I could be totally fucking wrong about everything I’m about to write. This is mostly just me long form “tweeting into the ether” about my smart contract journey. Proceed with caution.

There’s a lending pool with a million DVT tokens in balance, offering flash loans for free.

If only there was a way to attack and stop the pool from offering flash loans …

You start with 100 DVT tokens in balance.

So from jump, this particular problem is pretty clear that we don’t need to necessarily try to drain the contract or anything but just stop it from offering flash loans. I’m not really sure how that’s done without bankrupting the contract, so let’s take a look at the code…

Checking out the Receiver contract for the flash loan and I’m not noticing anything too wonky here…

Here we just set the owner of the contract to match the address that deployed the contract initially as well as set the pool (of type UnstoppableLender) to point to the contract we’re hoping to freeze. Nothing special to see here (or is there?).

This is an external function meant to be called by UnstoppableLender in order to move tokens to the ReceiverUnstoppable , specifying two arguments the tokenAddress which is basically the address of the contract holding the state for a particular ERC20 that we are trying to transfer to Receiver , and an amount that details exactly how many tokens we should be sending.

From the first line of this function, we see that in order for this function to not revert, the address that called this particular function must have come from the pool variable we set up in the constructor.

Sidenote: the transfer method is a method that looks like this for any ERC20, where we can move value from the contract to whichever address we specify _to with an appropriate _value and returns a bool indicating if the call was successful or not.

function transfer(address _to, uint256 _value) public returns (bool success)

After we satisfy the first condition, we move onto the second requirement that casts the ERC20 standard over the provided tokenAddress in the arguments (to ensure we have access to that transfer f(x) you see up there).

So this seems to be pretty on par with a flash loan recipients’ obligations to their lender. As long as we’re getting tokens from the DVT pool and then sending them back at the end all of the state changes we attempt with this flash loan should be accepted.

So this looks like a f(x) that’s meant to be called by the owner of the ReceiverUnstoppable contract that was set in the constructor and it only expects an amount to request in the form of a flash loan. The final line of this function appears to call on the pool object that’s pointing at the DVT contract to make a request for some ⚡️ loans.

I’m seeing a lot of OpenZeppelin standards at the top which gives me very little faith that I’ll be draining this contract completely. I’m pretty new to this but typically seeing these libraries means I’m not gonna be able to rely on reentrancy or arithmetic bs here. There doesn’t seem to be anything about ownership of contract though… 🧐

I’m also noticing that this contract seems to have an interface for the receiveTokens f(x) that’s written out in our other contract of concern for this challenge.

So a few things, we’re using SafeMath to manage arithmetic and try to stop overflows, so it’s not likely that we’ll be able to fuck off poolBalance which is of type uint256, and then we have public access to the DamnValuableToken ERC20 implementation as well as the poolBalance that seems pretty well guarded although visible.

Here, we just want to provide a token address that should point to the location of the DamnValuableToken ERC20 implementation and make sure it’s not pointing to the null address address(0) at which point we point our DamnValuableToken at the tokenAddress provided by msg.sender when they deployed the function.

Something about the use of a wrapper deposit function instead of using native functions within ERC20 standard feels intuitively off to me, but I’m also pretty ignorant as to what current best practices are for most shit like this right now. But this particular wrapper is expecting an amount to be provided by the address that calls this function and we are doing a check immediately to make sure that msg.sender isn’t calling this with zero tokens to deposit.

Sidenote: the transferFrom method is again part of the ERC20 standard, that differentiates from the previously inspected transfer only in that we are not explicitly deciding which address the value should be sourced from. Notice that in this particular call we need to specify this because we are transferring value between users within the DamnValuableToken contract, so we need to know both parties in advance and cannot implicitly derive this from msg.sender . F(x) returns bool on success but should throw unless the _from account has deliberately authorized the sender of the message via some mechanism

function transferFrom(address _from, address _to, uint256 _value) public returns (bool success)

Then the poolBalance should be incremented to reflect the updated balance after you call this function as the last step.

This is pretty weird because I don’t see any clear fallback function or anything that will manage incrementing poolBalance if we decide to send/transfer funds to UnstoppableLender through ERC20 methods rather than using this depositTokens wrapper. That seems weird but maybe this is a normal thing.

This seems to be the last stop on our journey and I’m seeing a lot of small clues being confirmed about flashLoan from up until this point. We could’ve guessed it was external or public since we were calling it from within our ReceiverUnstoppable contract, and I assumed it would be guarded against reentrancy which turns out to be a good assumption.

And it looks like this function is taking in an amount argument which we could infer from the fact that we supplied an argument by this name from within ReceiverUnstoppable when we make the call to executeFlashLoan

This f(x) is giving me an odd tingly sensation in my brain as I’m scanning it, I’m pretty certain it’s just me feeling excitement that the error must be in here through some sort of process of elimination.

So we have a quick check to make sure that we aren’t trying to request a loan for zilch (although I wonder what kinda attack surface that enables), and then we get a read on how many DVT tokens our current contract holds by checking its state within DVT.

Sidenote: balanceOf is another method that can be found as part of the ERC20 standard and it’s a pretty straightforward public view function that lets us specify an address that we would like to see the quantity of tokens within the contract allocated to them

function balanceOf(address _owner) public view returns (uint256 balance)

So boom, we check out balance by passing in address(this) and make sure that the UnstoppableLender actually has enough tokens to fulfill their part of the obligation for the ⚡️ loan…. and the catch is on line 36.

We are asserting that poolBalance should match the reported state from balanceBefore — it’s really important to note here that balanceBefore is reading state directly from DVT contract while poolBalance is attempting to map directly to that balanceBefore variable. I think there are two ways I can stop the function given this line of code:

1. Use transfer or send methods to send one of our 10 DVT tokens to either the DVT contract specifying that we should increase the balance for UnstoppableLender’s address and so balanceBefore shouldn’t match with poolBalance after we’re done.

2. Same thing as (1) but sending DVT to UnstoppableLender contract directly (without using the depositTokens so that poolBalance doesn’t get incremented by value) and in this case poolBalance should be ahead of balanceBefore

In either of these two cases, our assert should fail and flash loans should no longer be possible.

Going forward, it looks like if the assertion clears then we are going to hit the DVT contract and ask it to transfer over tokens to msg.sender ’s address for the total borrow amount. Once that transfer is made and approved, we then call the receiveTokens interface with the msg.sender as the target to receive the tokens which are returned within the same call.

Once we make exit the IReceiver call then we should have received back full payment for the loan which is verified by the last two lines of the the smart contract that are checking the balance after the flash loan has been provided. If the balance hasn’t been paid back all of the changes associated with these function calls would be reverted and the gas from sender would be lost in the ether :(

The exploit method snippet depicted

So this was all we had to do to solve this one. Just shoot a token to the pool.

This was pretty fun for a first challenge. One thing that struck a personal cord for me with this one is to really reason about your safety checks and what exactly you’re trying to protect especially within functions that are making calls to potentially multiple other contracts/functions. It was kind of daunting just sitting on line 36 for a few minutes to really think about why my brain started freaking out a bit when it saw the line. I can think of a few workarounds for this one that might work, in order of my level of faith in the revision being functional:

1. We change the poolBalance value to instead reflect address(this).balance , so we are always operating with the current total balance in our UnstoppableLender and make sure that it matches with balanceBefore if we absolutely *must *check to be sure that separate states are matching

2. I probably wouldn’t even bother with trying to match a separate state within UnstoppableLender — as far as I can tell, we could’ve just checked to be sure balanceBefore wasn’t 0 and proceeded from there, but I could also be missing something in my analysis as to why we need to have the poolBalance mapping to DVT balance

3. Write a fallback function that updates poolBalance anytime money is deposited into the contract. Something in my gut tells me this is bad, but I’ll have to experiment later

Thanks for sifting through this if you made it to the end. I don’t normally write out stuff like this, but I’m trying a new thing; and I would love to hear more feedback on how I can make this sort of shit more accessible and useful to others or just general improvements/critiques/comments/whatever. \n \n This is literally just a brain dump as I’m trying to learn more about this stuff and I really really appreciate any feedback or insights into what I got wrong or what I should revisit my mental model for!