DeFiFoundry
50,000 USDC
View results
Submission Details
Severity: low
Invalid

Incorrect Position Existence Check Due to USD Value Use in getPositionInfo

Summary

The VaultReader::getPositionInfo function incorrectly uses the position's USD value to check if a position exists, leading to potential miscalculations and incorrect handling of positions.

Vulnerability Details

In the VaultReader::getPositionInfo function, the sizeInTokens variable is set to getPositionSizeInUsd which is incorrect. Also the getPositionInfo function, checks if the position's size is zero using sizeInTokens variable. However, this function returns the position size in USD, not tokens. The variable sizeInTokens is incorrectly assigned this USD value, leading to incorrect early exits. Even if getPositionSizeInUsd(key) does not return zero, it still results in incorrect behavior because the actual token size is never fetched or considered. The function does not call getPositionSizeInTokens, which would return the correct token amount. This causes positions with non-zero token sizes to be improperly handled based on a misleading USD value, leading to potential miscalculations and invalid data being returned.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.4;
import "forge-std/Test.sol";
import "../../contracts/VaultReader.sol";
contract VaultReaderTest is Test {
VaultReader vaultReader;
IDataStore dataStore;
function setUp() public {
// Deploy mock DataStore
dataStore = IDataStore(address(new MockDataStore()));
// Deploy VaultReader with mock addresses
vaultReader = new VaultReader(
address(0), // orderHandler (mock)
address(dataStore),
address(0), // orderVault (mock)
address(0), // reader (mock)
address(0) // referralStorage (mock)
);
}
// Fuzz test to show sizeInUsd ≠ sizeInTokens
function testFuzz_PositionSizeComparison(uint256 sizeInUsd, uint256 sizeInTokens) public {
vm.assume(sizeInUsd != sizeInTokens);
bytes32 positionKey = keccak256(abi.encode("test_position"));
// Mock DataStore to return fuzzed values
MockDataStore(address(dataStore)).setUint(
keccak256(abi.encode(positionKey, vaultReader.SIZE_IN_USD())),
sizeInUsd
);
MockDataStore(address(dataStore)).setUint(
keccak256(abi.encode(positionKey, vaultReader.SIZE_IN_TOKENS())),
sizeInTokens
);
// Get values from VaultReader
uint256 actualSizeInUsd = vaultReader.getPositionSizeInUsd(positionKey);
uint256 actualSizeInTokens = vaultReader.getPositionSizeInTokens(positionKey);
// Assert mismatch between USD and token sizes
assertTrue(
actualSizeInUsd != actualSizeInTokens,
"sizeInUsd should never equal sizeInTokens"
);
}
}
// Mock DataStore implementation
contract MockDataStore is IDataStore {
mapping(bytes32 => uint256) private uints;
mapping(bytes32 => address) private addresses;
mapping(bytes32 => bool) private bools;
function setUint(bytes32 key, uint256 value) external {
uints[key] = value;
}
function getUint(bytes32 key) external view returns (uint256) {
return uints[key];
}
function getBool(bytes32 key) external view returns (bool) {
return bools[key];
}
function getAddress(bytes32 key) external view returns (address) {
return addresses[key];
}
function getBytes32ValuesAt(
bytes32, /* setKey */
uint256, /* start */
uint256 /* end */
) external pure returns (bytes32[] memory) {
return new bytes32[](0);
}
}

Output:

Ran 1 test for test/fuzzing/TestingUsdtoken.t.sol:VaultReaderTest
[PASS] testFuzz_PositionSizeComparison(uint256,uint256) (runs: 256, μ: 66012, ~: 66945)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 42.82ms (41.90ms CPU time)
Ran 1 test suite in 45.92ms (42.82ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Which means you can't set sizeInTokens variable to getPositionSizeInUsd because it will give wrong value.

Impact

  • Positions with actual token holdings but zero USD value are improperly ignored.

  • Downstream logic relying on getPositionInfo (e.g., liquidation checks, PnL calculations) may fail to process valid positions.

  • Users might see incorrect position statuses, leading to unintended financial outcomes.

Tools Used

  • Manual code review

Recommendations

Replace getPositionSizeInUsd(key) with getPositionSizeInTokens(key) in the sizeInTokens assignment to correctly check the position's existence based on token size:

function getPositionInfo(...) external view returns (...) {
uint256 sizeInTokens = getPositionSizeInTokens(key); // Fix: Use token size check
if (sizeInTokens == 0) {
return PositionData(...);
}
// Rest of the logic
}
Updates

Lead Judging Commences

n0kto Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

invalid_getPosition_sizeInTokens_value_in_USD

Only check if there are no tokens. Checking if USD is 0 is equivalent. There is no problem here, even if the variable has an incorrect name: Informational.

Support

FAQs

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