search

code-423n4 / 2023-08-dopex-findings

3 stars 3 forks source link

[M-11] Out of bounds array access with integer overflow vulnerability in PerpetualAtlanticVault contract #591

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L243-L250 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L346-L351 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L429-L437 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L328 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L413 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L315-L369

Vulnerability details

Impact

Detailed description of the impact of this finding. There are several issues that lead to out of bounds array access here and lead to integer overflow vulnerability in the settle function within the PerpetualAtlanticVault contract, they are:

  1. On line 413 strikes.length is not initially set to a value and is used in a loop.
  2. On line 328 optionIds.length is initially set to null or empty and is also used in a loop.

The function called setLpAllowance posses an out of bounds array access. So, the value in the function does not fall within the max length of the array, it appears to be set to zero or nothing.

This means that the functions the variable arrays are in are vulnerable to attacks like integer overflow whichI will demonstrate below in an exploit.

Proof of Concept

Exploits Foundry settle function integer overflow

// https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L315-L369

function testSettle() external payable {
    setApprovals(address(1));
    setApprovals(address(2));
    setApprovals(address(3));

    mintWeth(5 ether, address(1));
    mintWeth(20 ether, address(2));
    mintWeth(25 ether, address(3));

    deposit(5 ether, address(1));
    deposit(20 ether, address(2));
    deposit(25 ether, address(3));

    uint256 userBalance = vaultLp.balanceOf(address(1));

 vm.expectRevert(stdError.arithmeticError);
    uint256[] memory datae = new uint256[](1);
    datae[0] = uint256(1)+uint256(115792089237316195423570985008687907853269984665640564039457584007913129639935);
    vault.settle(datae);

}

Test Case

  1. Deploy code above in testIntegration() function within the tests/perp-vault/Integration.t.sol contract.
  2. Then in terminal run: forge test -vvv --match-path "tests/perp-vault/Integration.t.sol" --match-test "testSettle"
  3. Integer overflow is affected as depicted in log below. Log Foundry 
    
2023-08-dopex % forge test -vvvv --match-path "tests/perp-vault/Integration.t.sol" --match-test "testSettle" 
[⠃] Compiling...
[⠒] Compiling 1 files with 0.8.19
[⠑] Solc 0.8.19 finished in 5.41s
Compiler run successful with warnings:
Warning (2072): Unused local variable.
--> tests/perp-vault/Integration.t.sol:350:5:
|
350 |     uint256 userBalance = vaultLp.balanceOf(address(1));
|     ^^^^^^^^^^^^^^^^^^^

Warning (2072): Unused local variable. --> tests/perp-vault/Integration.t.sol:378:5: | 378 | uint256 userBalance = vaultLp.balanceOf(address(1)); | ^^^^^^^^^^^^^^^^^^^

Running 1 test for tests/perp-vault/Integration.t.sol:Integration [PASS] testSettle() (gas: 612942) Traces: [612942] Integration::testSettle() ├─ [0] VM::startPrank(0x0000000000000000000000000000000000000001, 0x0000000000000000000000000000000000000001) │ └─ ← () ├─ [24681] MockToken::approve(PerpetualAtlanticVault: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000000001, spender: PerpetualAtlanticVault: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211], amount: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000001 ├─ [24681] MockToken::approve(PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000000001, spender: PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], amount: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000001 ├─ [24681] MockToken::approve(PerpetualAtlanticVault: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000000001, spender: PerpetualAtlanticVault: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211], amount: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000001 ├─ [24681] MockToken::approve(PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000000001, spender: PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], amount: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000001 ├─ [0] VM::stopPrank() │ └─ ← () ├─ [0] VM::startPrank(0x0000000000000000000000000000000000000002, 0x0000000000000000000000000000000000000002) │ └─ ← () ├─ [24681] MockToken::approve(PerpetualAtlanticVault: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000000002, spender: PerpetualAtlanticVault: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211], amount: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000001 ├─ [24681] MockToken::approve(PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000000002, spender: PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], amount: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000001 ├─ [24681] MockToken::approve(PerpetualAtlanticVault: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000000002, spender: PerpetualAtlanticVault: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211], amount: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000001 ├─ [24681] MockToken::approve(PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000000002, spender: PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], amount: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000001 ├─ [0] VM::stopPrank() │ └─ ← () ├─ [0] VM::startPrank(0x0000000000000000000000000000000000000003, 0x0000000000000000000000000000000000000003) │ └─ ← () ├─ [24681] MockToken::approve(PerpetualAtlanticVault: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000000003, spender: PerpetualAtlanticVault: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211], amount: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000001 ├─ [24681] MockToken::approve(PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000000003, spender: PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], amount: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000001 ├─ [24681] MockToken::approve(PerpetualAtlanticVault: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000000003, spender: PerpetualAtlanticVault: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211], amount: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000001 ├─ [24681] MockToken::approve(PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000000003, spender: PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], amount: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000001 ├─ [0] VM::stopPrank() │ └─ ← () ├─ [34093] MockToken::mint(0x0000000000000000000000000000000000000001, 5000000000000000000 [5e18]) │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: 0x0000000000000000000000000000000000000001, amount: 5000000000000000000 [5e18]) │ └─ ← () ├─ [25293] MockToken::mint(0x0000000000000000000000000000000000000002, 20000000000000000000 [2e19]) │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: 0x0000000000000000000000000000000000000002, amount: 20000000000000000000 [2e19]) │ └─ ← () ├─ [25293] MockToken::mint(0x0000000000000000000000000000000000000003, 25000000000000000000 [2.5e19]) │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: 0x0000000000000000000000000000000000000003, amount: 25000000000000000000 [2.5e19]) │ └─ ← () ├─ [0] VM::startPrank(0x0000000000000000000000000000000000000001, 0x0000000000000000000000000000000000000001) │ └─ ← () ├─ [143192] PerpetualAtlanticVaultLP::deposit(5000000000000000000 [5e18], 0x0000000000000000000000000000000000000001) │ ├─ [7644] PerpetualAtlanticVault::getUnderlyingPrice() [staticcall] │ │ ├─ [2324] MockRdpxEthPriceOracle::getRdpxPriceInEth() [staticcall] │ │ │ └─ ← 20000000 [2e7] │ │ └─ ← 20000000 [2e7] │ ├─ [51946] PerpetualAtlanticVault::updateFunding() │ │ ├─ [7315] MockToken::transfer(PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], 0) │ │ │ ├─ emit Transfer(from: PerpetualAtlanticVault: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211], to: PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], amount: 0) │ │ │ └─ ← true │ │ ├─ [4031] PerpetualAtlanticVaultLP::addProceeds(0) │ │ │ ├─ [583] MockToken::balanceOf(PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c]) [staticcall] │ │ │ │ └─ ← 0 │ │ │ └─ ← () │ │ ├─ emit FundingPaid(sender: PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], amount: 0, latestFundingPaymentPointer: 0) │ │ └─ ← () │ ├─ [18961] MockToken::transferFrom(0x0000000000000000000000000000000000000001, PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], 5000000000000000000 [5e18]) │ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000001, to: PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], amount: 5000000000000000000 [5e18]) │ │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000001 │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: 0x0000000000000000000000000000000000000001, amount: 5000000000000000000 [5e18]) │ ├─ emit Deposit(caller: 0x0000000000000000000000000000000000000001, owner: 0x0000000000000000000000000000000000000001, assets: 5000000000000000000 [5e18], shares: 5000000000000000000 [5e18]) │ └─ ← 0x0000000000000000000000000000000000000000000000004563918244f40000 ├─ [0] VM::stopPrank() │ └─ ← () ├─ [0] VM::startPrank(0x0000000000000000000000000000000000000002, 0x0000000000000000000000000000000000000002) │ └─ ← () ├─ [36826] PerpetualAtlanticVaultLP::deposit(20000000000000000000 [2e19], 0x0000000000000000000000000000000000000002) │ ├─ [1144] PerpetualAtlanticVault::getUnderlyingPrice() [staticcall] │ │ ├─ [324] MockRdpxEthPriceOracle::getRdpxPriceInEth() [staticcall] │ │ │ └─ ← 20000000 [2e7] │ │ └─ ← 20000000 [2e7] │ ├─ [11472] PerpetualAtlanticVault::updateFunding() │ │ ├─ [3315] MockToken::transfer(PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], 0) │ │ │ ├─ emit Transfer(from: PerpetualAtlanticVault: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211], to: PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], amount: 0) │ │ │ └─ ← true │ │ ├─ [2031] PerpetualAtlanticVaultLP::addProceeds(0) │ │ │ ├─ [583] MockToken::balanceOf(PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c]) [staticcall] │ │ │ │ └─ ← 5000000000000000000 [5e18] │ │ │ └─ ← () │ │ ├─ emit FundingPaid(sender: PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], amount: 0, latestFundingPaymentPointer: 0) │ │ └─ ← () │ ├─ [3041] MockToken::transferFrom(0x0000000000000000000000000000000000000002, PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], 20000000000000000000 [2e19]) │ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000002, to: PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], amount: 20000000000000000000 [2e19]) │ │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000001 │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: 0x0000000000000000000000000000000000000002, amount: 20000000000000000000 [2e19]) │ ├─ emit Deposit(caller: 0x0000000000000000000000000000000000000002, owner: 0x0000000000000000000000000000000000000002, assets: 20000000000000000000 [2e19], shares: 20000000000000000000 [2e19]) │ └─ ← 0x000000000000000000000000000000000000000000000001158e460913d00000 ├─ [0] VM::stopPrank() │ └─ ← () ├─ [0] VM::startPrank(0x0000000000000000000000000000000000000003, 0x0000000000000000000000000000000000000003) │ └─ ← () ├─ [36826] PerpetualAtlanticVaultLP::deposit(25000000000000000000 [2.5e19], 0x0000000000000000000000000000000000000003) │ ├─ [1144] PerpetualAtlanticVault::getUnderlyingPrice() [staticcall] │ │ ├─ [324] MockRdpxEthPriceOracle::getRdpxPriceInEth() [staticcall] │ │ │ └─ ← 20000000 [2e7] │ │ └─ ← 20000000 [2e7] │ ├─ [11472] PerpetualAtlanticVault::updateFunding() │ │ ├─ [3315] MockToken::transfer(PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], 0) │ │ │ ├─ emit Transfer(from: PerpetualAtlanticVault: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211], to: PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], amount: 0) │ │ │ └─ ← true │ │ ├─ [2031] PerpetualAtlanticVaultLP::addProceeds(0) │ │ │ ├─ [583] MockToken::balanceOf(PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c]) [staticcall] │ │ │ │ └─ ← 25000000000000000000 [2.5e19] │ │ │ └─ ← () │ │ ├─ emit FundingPaid(sender: PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], amount: 0, latestFundingPaymentPointer: 0) │ │ └─ ← () │ ├─ [3041] MockToken::transferFrom(0x0000000000000000000000000000000000000003, PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], 25000000000000000000 [2.5e19]) │ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000003, to: PerpetualAtlanticVaultLP: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], amount: 25000000000000000000 [2.5e19]) │ │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000001 │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: 0x0000000000000000000000000000000000000003, amount: 25000000000000000000 [2.5e19]) │ ├─ emit Deposit(caller: 0x0000000000000000000000000000000000000003, owner: 0x0000000000000000000000000000000000000003, assets: 25000000000000000000 [2.5e19], shares: 25000000000000000000 [2.5e19]) │ └─ ← 0x0000000000000000000000000000000000000000000000015af1d78b58c40000 ├─ [0] VM::stopPrank() │ └─ ← () ├─ [564] PerpetualAtlanticVaultLP::balanceOf(0x0000000000000000000000000000000000000001) [staticcall] │ └─ ← 5000000000000000000 [5e18] ├─ [0] VM::expectRevert(Arithmetic over/underflow) │ └─ ← () └─ ← "Arithmetic over/underflow"

Test result: ok. 1 passed; 0 failed; finished in 839.01ms

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.
```sol
// https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L243-L250
  function setLpAllowance(bool increase) external onlyRole(DEFAULT_ADMIN_ROLE) {
    increase
      ? collateralToken.approve(
        addresses.perpetualAtlanticVaultLP,
        type(uint256).max
      )
      : collateralToken.approve(addresses.perpetualAtlanticVaultLP, 0);
  }
// https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L346-L351
    // Transfer collateral token from perpetual vault to rdpx rdpxV2Core
    collateralToken.safeTransferFrom(
      addresses.perpetualAtlanticVaultLP,
      addresses.rdpxV2Core,
      ethAmount
    );
// https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L429-L437
      uint256 premium = calculatePremium(
        strike,
        amount,
        timeToExpiry,
        getUnderlyingPrice()
      );

      latestFundingPerStrike[strike] = latestFundingPaymentPointer;
      fundingAmount += premium;
// https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L328
    for (uint256 i = 0; i < optionIds.length; i++) {
// https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L413
    for (uint256 i = 0; i < strikes.length; i++) {

Tools Used

VS Code. Foundry. Mythx.

Recommended Mitigation Steps

Adjust the code so that all values fall within the range. Initialise the variable of optionsIds and strikes to a non zero value.

Assessed type

Under/Overflow

bytes032 commented 1 year ago

Invalid

c4-pre-sort commented 1 year ago

bytes032 marked the issue as low quality report

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Invalid