Share Dialog

Share Dialog

<100 subscribers

<100 subscribers

Damn Vulnerable DeFi: Naive Receiver

Damn Vulnerable DeFi: Naive Receiver

Note: constant and immutable keywords both specify a type of state variable on the Ethereum blockchain. constant keywords can’t be changed and must be set at creation while immutable can be changed within the constructor at deployment. Also, EVM doesn’t reserve storage for variables that are given these flags and instead just puts their value in place where the variable is called. (Not really sure what this really means yet will update later).

This is pretty straightforward, we have an external function that doesn’t do anything but read and return the FIXED_FEE constantvariable that was defined earlier in the contract. Not much to gauge from this.

This shit is nonReentrant. Okay so any hopes I had initially of calling flashLoan multiple times to bankrupt the contract might be for naught. This f(x) is going to be called externally and it’s expecting just two arguments that it’ll process — namely, a payable address and a uint256 known respectively as borrower and the borrowAmount and it does a couple of important checks right out the gate with these two pieces of info.

First, we see that the f(x) is checking the current balance of the pool and then verifying that the amount being requested as the borrowAmount isn’t more than what’s actually available within the contract. Next, the f(x) is going to do a check on the borrower that was provided to ensure that this function is being called by a contract.

Sidenote: The isContractmethod relies on extcodesize, which returns 0 for contracts in construction, since the code is only stored at the end of the constructor execution. EXTCODESIZE returns 0 if it is called from the constructor of a contract. So if you are using this in a security sensitive setting, you would have to consider if this is a problem. ***It is unsafe to assume that an address for which this function returns false is an externally-owned account (EOA) and not a contract. Among others, isContract will return false for the following types of addresses: \n — an externally-owned account \n — a contract in construction \n — an address where a contract will be created \n — an address where a contract lived, but was destroyed

After all of those checks clear out, the contract attempts to make a call to the borrower contract with a call that attempts to call a f(x) on the other end by the name of receiveEther that accepts a uint256 as an argument with the NaiveReceiverLendingPoolpassing in its FIXED_FEE as the argument.

We do two final checks for this f(x) where we check if success of our external call is true and then we make sure that we were given our refund for the flashLoan at the end which will either succeed and everything is squared away or it fails and all the changes get reverted.

Nothing too big jumps out from any of this so far, but I’m very curious to see the f(x) on the other end that’s handling reception of the ETH as well as the payment of the fee.

This just looks like a basic fallback function for allowing the contract to accept deposits. No logic is found in the function, so I won’t spend too much time thinking about this one.

FlashLoanReceiver.sol

Ah, beautiful. There doesn’t seem to be any sort of guard for reentrancy imported here. Maybe we can use that? We are still using SafeMath and Address but this time we’re specifically using it for address payable and then immediately there’s this address payable private pool state that doesn’t get an instantiation from jump.

Here we have a public constructor that is just taking address payable pool as its argument and setting pool to be an instance of the contract that’s being pointed to with the poolAddress variable, in our case the NaiveReceiverLenderPool

This is what we came for. We have a function that can be called by anyone to send value to this smart contract, and we also attach a fee argument here that maps to what we passed in as our FIXED_FEE and immediately makes a check to be sure that the address that is sending this call is matching the pool instance that was created in the constructor when the contract was deployed. We see that the f(x) creates amountToBeRepaid that takes whatever value is sent over for the loan request plus a fee of 1ETH as defined by FIXED_FEE , and then we make a quick check to be certain that the amount requested in addition to the FIXED_FEE is available in the contract to actually pay back the NaiveReceiverLenderPool at the end of this call. Some action is executed and then the money is returned to the lending pool; however, the issue here is that the Receiver has absolutely no way to reject incoming loans that are called with their address. What this means is that, when we look at NaiveReceiverLenderPool we can see that the function to call flashLoan is only flagged to be called externally (by anyone) and we can supply an address of our choosing to accept the flashLoan by passing it into borrower , and in this case we can bankrupt Receiver entirely by asking for flash loans that are getting hit with a fee.

This is just some general internal f(x) that only Receiver can call in order to pull off whatever trading insight they believe they’ve reasoned upon. Nothing to see here beyond the confines of your own imagination.

A fallback for just depositing money into the contract.

So the solution here is to just call flashLoan from our wallet, and we don’t even have to specify a loan amount as long as we have a 1 ETH FIXED_FEE , so we just drain the contract with 10 calls. I’m going to play around with the draining the contract in 1 transaction; I have a feeling there are a few options.

This one made me feel bad 😞 I didn’t like draining some random person’s funds that’s cruel, but I did learn quite a bit about securing your contracts and making sure that receiving ether from a contract is always a more explicit flow for the Receiver — a few things I’d have done differently:

1. A check within the flashLoan f(x) inside of NaiveReceiverLendingPoolto be sure that borrower is msg.sender would be one change I’d make here to ensure that we aren’t actually letting random people request loans for others.

2. We should probably add a check in this current FlashLoanReceiver to make sure that the trade was profitable or else we should revert like so require(savedAmountFromExecuteFlashLoan > amountToBeRepaid + gasLimit) This way we have the benefit of simply reverting which will cause success in NaiveReceiverLendingPool to be false which will cause the following require clause to fail.

This one was mostly just about considering the effects of withdrawal design and how dangerous it can be to not affirm origins of f(x) calls.

Note: constant and immutable keywords both specify a type of state variable on the Ethereum blockchain. constant keywords can’t be changed and must be set at creation while immutable can be changed within the constructor at deployment. Also, EVM doesn’t reserve storage for variables that are given these flags and instead just puts their value in place where the variable is called. (Not really sure what this really means yet will update later).

This is pretty straightforward, we have an external function that doesn’t do anything but read and return the FIXED_FEE constantvariable that was defined earlier in the contract. Not much to gauge from this.

This shit is nonReentrant. Okay so any hopes I had initially of calling flashLoan multiple times to bankrupt the contract might be for naught. This f(x) is going to be called externally and it’s expecting just two arguments that it’ll process — namely, a payable address and a uint256 known respectively as borrower and the borrowAmount and it does a couple of important checks right out the gate with these two pieces of info.

First, we see that the f(x) is checking the current balance of the pool and then verifying that the amount being requested as the borrowAmount isn’t more than what’s actually available within the contract. Next, the f(x) is going to do a check on the borrower that was provided to ensure that this function is being called by a contract.

Sidenote: The isContractmethod relies on extcodesize, which returns 0 for contracts in construction, since the code is only stored at the end of the constructor execution. EXTCODESIZE returns 0 if it is called from the constructor of a contract. So if you are using this in a security sensitive setting, you would have to consider if this is a problem. ***It is unsafe to assume that an address for which this function returns false is an externally-owned account (EOA) and not a contract. Among others, isContract will return false for the following types of addresses: \n — an externally-owned account \n — a contract in construction \n — an address where a contract will be created \n — an address where a contract lived, but was destroyed

After all of those checks clear out, the contract attempts to make a call to the borrower contract with a call that attempts to call a f(x) on the other end by the name of receiveEther that accepts a uint256 as an argument with the NaiveReceiverLendingPoolpassing in its FIXED_FEE as the argument.

We do two final checks for this f(x) where we check if success of our external call is true and then we make sure that we were given our refund for the flashLoan at the end which will either succeed and everything is squared away or it fails and all the changes get reverted.

Nothing too big jumps out from any of this so far, but I’m very curious to see the f(x) on the other end that’s handling reception of the ETH as well as the payment of the fee.

This just looks like a basic fallback function for allowing the contract to accept deposits. No logic is found in the function, so I won’t spend too much time thinking about this one.

FlashLoanReceiver.sol

Ah, beautiful. There doesn’t seem to be any sort of guard for reentrancy imported here. Maybe we can use that? We are still using SafeMath and Address but this time we’re specifically using it for address payable and then immediately there’s this address payable private pool state that doesn’t get an instantiation from jump.

Here we have a public constructor that is just taking address payable pool as its argument and setting pool to be an instance of the contract that’s being pointed to with the poolAddress variable, in our case the NaiveReceiverLenderPool

This is what we came for. We have a function that can be called by anyone to send value to this smart contract, and we also attach a fee argument here that maps to what we passed in as our FIXED_FEE and immediately makes a check to be sure that the address that is sending this call is matching the pool instance that was created in the constructor when the contract was deployed. We see that the f(x) creates amountToBeRepaid that takes whatever value is sent over for the loan request plus a fee of 1ETH as defined by FIXED_FEE , and then we make a quick check to be certain that the amount requested in addition to the FIXED_FEE is available in the contract to actually pay back the NaiveReceiverLenderPool at the end of this call. Some action is executed and then the money is returned to the lending pool; however, the issue here is that the Receiver has absolutely no way to reject incoming loans that are called with their address. What this means is that, when we look at NaiveReceiverLenderPool we can see that the function to call flashLoan is only flagged to be called externally (by anyone) and we can supply an address of our choosing to accept the flashLoan by passing it into borrower , and in this case we can bankrupt Receiver entirely by asking for flash loans that are getting hit with a fee.

This is just some general internal f(x) that only Receiver can call in order to pull off whatever trading insight they believe they’ve reasoned upon. Nothing to see here beyond the confines of your own imagination.

A fallback for just depositing money into the contract.

So the solution here is to just call flashLoan from our wallet, and we don’t even have to specify a loan amount as long as we have a 1 ETH FIXED_FEE , so we just drain the contract with 10 calls. I’m going to play around with the draining the contract in 1 transaction; I have a feeling there are a few options.

This one made me feel bad 😞 I didn’t like draining some random person’s funds that’s cruel, but I did learn quite a bit about securing your contracts and making sure that receiving ether from a contract is always a more explicit flow for the Receiver — a few things I’d have done differently:

1. A check within the flashLoan f(x) inside of NaiveReceiverLendingPoolto be sure that borrower is msg.sender would be one change I’d make here to ensure that we aren’t actually letting random people request loans for others.

2. We should probably add a check in this current FlashLoanReceiver to make sure that the trade was profitable or else we should revert like so require(savedAmountFromExecuteFlashLoan > amountToBeRepaid + gasLimit) This way we have the benefit of simply reverting which will cause success in NaiveReceiverLendingPool to be false which will cause the following require clause to fail.

This one was mostly just about considering the effects of withdrawal design and how dangerous it can be to not affirm origins of f(x) calls.