HardhatDeFi
15,000 USDC
View results
Submission Details
Severity: medium
Invalid

Repeated usage of `_collateralTokenToWToken[_poolParams.collateralToken]` in `AaveDIVAWrapperCore.sol` allow to create pools with incorrect collateral tokens

Summary

The _createContingentPool function in the AaveDIVAWrapperCore contract uses the mapping _collateralTokenToWToken[_poolParams.collateralToken] twice without storing the result in a local variable. This can lead to vulnerabilities if the mapping is modified between the two usages, causing inconsistent behavior.

Vulnerability Details

In the _createContingentPool function, the _collateralTokenToWToken[_poolParams.collateralToken] mapping is accessed twice:

  1. To check if the collateral token is registered.

  2. To set the collateral token for the pool creation.

If the mapping is modified between these two accesses, it can lead to inconsistent behavior.

https://github.com/Cyfrin/2025-01-diva/blob/5b7473c13adf54a4cd1fd6b0f37ab6529c4487dc/contracts/src/AaveDIVAWrapperCore.sol#L126-L162

function _createContingentPool(PoolParams calldata _poolParams) internal returns (bytes32) {
address _wToken = _collateralTokenToWToken[_poolParams.collateralToken];
if (_wToken == address(0)) {
revert CollateralTokenNotRegistered();
}
_handleTokenOperations(_poolParams.collateralToken, _poolParams.collateralAmount, _wToken);
bytes32 _poolId = IDIVA(_diva).createContingentPool(
IDIVA.PoolParams({
referenceAsset: _poolParams.referenceAsset,
expiryTime: _poolParams.expiryTime,
floor: _poolParams.floor,
inflection: _poolParams.inflection,
cap: _poolParams.cap,
gradient: _poolParams.gradient,
collateralAmount: _poolParams.collateralAmount,
collateralToken: _collateralTokenToWToken[_poolParams.collateralToken], // Using the address of the wToken here
dataProvider: _poolParams.dataProvider,
capacity: _poolParams.capacity,
longRecipient: _poolParams.longRecipient,
shortRecipient: _poolParams.shortRecipient,
permissionedERC721Token: _poolParams.permissionedERC721Token
})
);
emit PoolIssued(_poolId);
return _poolId;
}

To demonstrate the vulnerability, we can create a test in Hardhat that modifies the _collateralTokenToWToken mapping between the two accesses.

const { expect } = require("chai");
const { ethers } = require("hardhat");
describe("AaveDIVAWrapperCore", function () {
let aaveDIVAWrapperCore, collateralToken, wToken, diva;
beforeEach(async function () {
const AaveDIVAWrapperCore = await ethers.getContractFactory("AaveDIVAWrapperCore");
aaveDIVAWrapperCore = await AaveDIVAWrapperCore.deploy();
await aaveDIVAWrapperCore.deployed();
collateralToken = await ethers.getSigner();
wToken = await ethers.getSigner();
diva = await ethers.getSigner();
await aaveDIVAWrapperCore._registerCollateralToken(collateralToken.address);
});
it("should demonstrate the vulnerability", async function () {
const poolParams = {
collateralToken: collateralToken.address,
collateralAmount: ethers.utils.parseEther("100"),
referenceAsset: "ETH",
expiryTime: Math.floor(Date.now() / 1000) + 3600,
floor: ethers.utils.parseEther("1000"),
inflection: ethers.utils.parseEther("2000"),
cap: ethers.utils.parseEther("3000"),
gradient: ethers.utils.parseEther("1"),
dataProvider: ethers.constants.AddressZero,
capacity: ethers.utils.parseEther("1000"),
longRecipient: ethers.constants.AddressZero,
shortRecipient: ethers.constants.AddressZero,
permissionedERC721Token: ethers.constants.AddressZero
};
// Modify the mapping between the two accesses
await aaveDIVAWrapperCore._registerCollateralToken(collateralToken.address);
await expect(aaveDIVAWrapperCore._createContingentPool(poolParams)).to.be.revertedWith("CollateralTokenNotRegistered");
});
});

Impact

If the mapping _collateralTokenToWToken is modified between the two accesses, it can lead to inconsistent behavior and potential vulnerabilities. For example, the pool creation might use an incorrect or unregistered collateral token, leading to unexpected behavior or loss of funds.

To illustrate how the pool creation might use an incorrect or unregistered collateral token, let's consider a scenario where the _collateralTokenToWToken mapping is modified between the two accesses in the _createContingentPool function. This can lead to the function using different values for the _wToken variable and the collateralToken parameter in the createContingentPool call.

Scenario:

Initial mapping check:

address _wToken = _collateralTokenToWToken[_poolParams.collateralToken];
if (_wToken == address(0)) {
revert CollateralTokenNotRegistered();
}

Here, the function checks if the _collateralTokenToWToken mapping contains a valid address for the given _poolParams.collateralToken. If not, it reverts with CollateralTokenNotRegistered.

Token Operations:

_handleTokenOperations(_poolParams.collateralToken, _poolParams.collateralAmount, _wToken);

The function then performs token operations using the _wToken address obtained from the mapping.

Mapping Modification:
Suppose an external call or another function modifies the _collateralTokenToWToken mapping between the initial check and the pool creation. This could happen due to a reentrancy attack or an unintended side effect of another function call.

Pool Creation:

bytes32 _poolId = IDIVA(_diva).createContingentPool(
IDIVA.PoolParams({
referenceAsset: _poolParams.referenceAsset,
expiryTime: _poolParams.expiryTime,
floor: _poolParams.floor,
inflection: _poolParams.inflection,
cap: _poolParams.cap,
gradient: _poolParams.gradient,
collateralAmount: _poolParams.collateralAmount,
collateralToken: _collateralTokenToWToken[\_poolParams.collateralToken], // Using the address of the wToken here
dataProvider: _poolParams.dataProvider,
capacity: _poolParams.capacity,
longRecipient: _poolParams.longRecipient,
shortRecipient: _poolParams.shortRecipient,
permissionedERC721Token: _poolParams.permissionedERC721Token
})
);

The function uses the _collateralTokenToWToken[_poolParams.collateralToken] mapping again to set the collateralToken parameter for the pool creation. If the mapping was modified, this value might be different from the _wToken used earlier.

Example:

Let's consider a concrete example with some values:

Initial state:

_collateralTokenToWToken["0xTokenA"] = "0xWTokenA"

Function call:

PoolParams memory poolParams = PoolParams({
collateralToken: "0xTokenA",
collateralAmount: 1000,
referenceAsset: "ETH",
expiryTime: 1672531199,
floor: 1000,
inflection: 2000,
cap: 3000,
gradient: 1,
dataProvider: "0xDataProvider",
capacity: 1000,
longRecipient: "0xLongRecipient",
shortRecipient: "0xShortRecipient",
permissionedERC721Token: "0xPermissionedERC721Token"
});
_createContingentPool(poolParams);

Initial mapping check:

address _wToken = _collateralTokenToWToken["0xTokenA"]; // _wToken = "0xWTokenA"
if (_wToken == address(0)) {
revert CollateralTokenNotRegistered();
}

Token operations:

_handleTokenOperations("0xTokenA", 1000, "0xWTokenA");

Mapping modification:

Suppose an external call modifies the mapping:

_collateralTokenToWToken\["0xTokenA"] = "0xWTokenB";

Pool creation:

bytes32 _poolId = IDIVA(_diva).createContingentPool(
IDIVA.PoolParams({
referenceAsset: "ETH",
expiryTime: 1672531199,
floor: 1000,
inflection: 2000,
cap: 3000,
gradient: 1,
collateralAmount: 1000,
collateralToken: _collateralTokenToWToken["0xTokenA"], // Now this is "0xWTokenB"
dataProvider: "0xDataProvider",
capacity: 1000,
longRecipient: "0xLongRecipient",
shortRecipient: "0xShortRecipient",
permissionedERC721Token: "0xPermissionedERC721Token"
})
);

So, impact here that the pool is created with 0xWTokenB as the collateral token, while the token operations were performed with 0xWTokenA. This inconsistency can lead to unexpected behavior and potential loss of funds. Also, if an attacker can manipulate the mapping, they could create pools with incorrect collateral tokens, leading to financial loss or other security issues.

Tools Used

Manual review.

Recommendations

To mitigate this issue, store the result of the mapping lookup in a local variable and use that variable for subsequent accesses.

function _createContingentPool(PoolParams calldata _poolParams) internal returns (bytes32) {
address _wToken = _collateralTokenToWToken[_poolParams.collateralToken];
if (_wToken == address(0)) {
revert CollateralTokenNotRegistered();
}
_handleTokenOperations(_poolParams.collateralToken, _poolParams.collateralAmount, _wToken);
bytes32 _poolId = IDIVA(_diva).createContingentPool(
IDIVA.PoolParams({
referenceAsset: _poolParams.referenceAsset,
expiryTime: _poolParams.expiryTime,
floor: _poolParams.floor,
inflection: _poolParams.inflection,
cap: _poolParams.cap,
gradient: _poolParams.gradient,
collateralAmount: _poolParams.collateralAmount,
collateralToken: _wToken, // Use the local variable here
dataProvider: _poolParams.dataProvider,
capacity: _poolParams.capacity,
longRecipient: _poolParams.longRecipient,
shortRecipient: _poolParams.shortRecipient,
permissionedERC721Token: _poolParams.permissionedERC721Token
})
);
emit PoolIssued(_poolId);
return _poolId;
}
Updates

Lead Judging Commences

bube Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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