Aave DIVA Wrapper
DIVA
HardhatDeFi
15,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

An incorrect parameter order in the constructor when initializing AaveDIVAWrapperCore makes the protocol unusable

avoloder

Summary

The incorrect order of the Diva and Aave addresses in the constructor of the AaveDIVAWrapperCore results in the addresses being assigned incorrectly within the contract.

Vulnerability Details

Contract AaveDIVAWrapper inherits from the abstract contract AaveDIVAWrapperCore where the addresses of the external protocols, i.e., Aave and DIVA are initialized. In the constructor of AaveDIVAWrapper we have the following order of arguments:

  1. address _aaveV3Pool

  2. address _diva

  3. address _owner

These are then passed to the constructor of AaveDIVAWrapperCore, as shown in the following code:

contract AaveDIVAWrapper is AaveDIVAWrapperCore, ReentrancyGuard {
/*//////////////////////////////////////////////////////////////
CONSTRUCTOR
//////////////////////////////////////////////////////////////*/
​
constructor(address _aaveV3Pool, address _diva, address _owner) AaveDIVAWrapperCore(_aaveV3Pool, _diva, _owner) {}
​
...
​
}

The problem is that the constructor of the AaveDIVAWrapperCore contract takes the arguments in the following order:

  1. address diva_

  2. address aaveV3Pool_

  3. address owner_

This means that the diva_ address and the aaveV3Pool_ address have been swapped, causing none of the functions to work and rendering the wrapper unusable.

The following code shows the assignment of the two addresses and the constructor of the AaveDIVAWrapperCore

/**
* @dev An abstract contract that inherits from `IAaveDIVAWrapper` and implements the core
* functions of the AaveDIVAWrapper contract as internal functions.
*/
abstract contract AaveDIVAWrapperCore is IAaveDIVAWrapper, Ownable2Step {
using SafeERC20 for IERC20Metadata;
​
/*//////////////////////////////////////////////////////////////
CONSTANTS
//////////////////////////////////////////////////////////////*/
​
// Addresses of DIVA Protocol and Aave V3 (Pool contract) that this contract interacts with.
// Set in the constructor and retrievable via `getContractDetails`.
address private immutable _diva;
address private immutable _aaveV3Pool; // Pool contract address
​
/*//////////////////////////////////////////////////////////////
STATE VARIABLES
//////////////////////////////////////////////////////////////*/
​
// Mappings between collateral tokens (e.g., USDC or USDT) to their corresponding wTokens, which are used as
// proxy collateral tokens in DIVA Protocol. Set by contract owner via `registerCollateralToken`.
mapping(address => address) private _collateralTokenToWToken;
mapping(address => address) private _wTokenToCollateralToken;
​
/*//////////////////////////////////////////////////////////////
CONSTRUCTOR
//////////////////////////////////////////////////////////////*/
​
/**
* @dev Initializes the AaveDIVAWrapper contract with the addresses of DIVA Protocol, Aave V3's Pool
* contract and the owner of the contract.
* @param diva_ Address of the DIVA Protocol contract.
* @param aaveV3Pool_ Address of the Aave V3 Pool contract.
* @param owner_ Address of the owner for the contract, who will be entitled to claim the yield.
* Retrievable via Ownable's `owner()` function or this contract's `getContractDetails` functions.
*/
constructor(address diva_, address aaveV3Pool_, address owner_) Ownable(owner_) {
// Validate that none of the input addresses is zero to prevent unintended initialization with default addresses.
// Zero address check on `owner_` is performed in the OpenZeppelin's `Ownable` contract.
if (diva_ == address(0) || aaveV3Pool_ == address(0)) {
revert ZeroAddress();
}
​
// Store the addresses of DIVA Protocol and Aave V3 in storage.
//@audit-issue wrong order -> diva_ and aaveV3Pool_ swapped
_diva = diva_;
_aaveV3Pool = aaveV3Pool_;
}
...
}


Furthermore, in the deployment script deployAaveDIVAWrapper.ts, we can also see that the order is

  1. AaveV3Pool

  2. DIVA

  3. Owner

// Deploy AaveDIVAWrapper
const AaveDIVAWrapper =
await hre.ethers.getContractFactory("AaveDIVAWrapper");
const aaveDIVAWrapper = await AaveDIVAWrapper.deploy(
AAVE_V3_POOL,
DIVA,
OWNER,
);
await aaveDIVAWrapper.waitForDeployment();

One would imagine that the tests would catch this mistake but if we look at the test file AaveDIVAWrapper.test.ts we see that in the moment of the deployment at L99 the order of the constructor arguments has been changed to

  1. divaAddress

  2. AaveAddress

  3. ownerAddress

// Deploy AaveDIVAWrapper contract
const aaveDIVAWrapper: AaveDIVAWrapper = await ethers.deployContract(
"AaveDIVAWrapper",
[divaAddress, aaveAddress, owner.address],
);
await aaveDIVAWrapper.waitForDeployment();
​

Impact

HIGH -> The wrapper will be deployed, but the addresses of the DIVA protocol and Aave will be swapped, meaning that any function relying on these external protocol will not work

POC

Change the order of the arguments when deploying AaveDIVAWrapper in AaveDIVAWrapper.test.ts and every test will fail:

// Deploy AaveDIVAWrapper contract
const aaveDIVAWrapper: AaveDIVAWrapper = await ethers.deployContract(
"AaveDIVAWrapper",
[aaveAddress, divaAddress, owner.address],
);
await aaveDIVAWrapper.waitForDeployment();

Tools Used

Manual Review

Recommendations

Change the argument order in the constructor of AaveDIVAWrapperCore to be the same as in the AaveDIVAWrapper, i.e.,

  1. aaveV3Pool_

  2. diva_

  3. owner_

Also, adapt the test file to ensure the order remains correct after the change.

Updates

Lead Judging Commences

bube Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Constructor arguments mismatch

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.