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

Improper Validation of `abi.decode` Data Leading to Invalid Protocol Handling

Summary :

  • The contract improperly decodes metadata using abi.decode and fails to perform sufficient validation on the extracted values. This could lead to incorrect protocol selection, potentially causing unintended behavior or security issues.

Vulnerability Details :

  • In the _runSwap function, the contract decodes metadata to extract the protocol and corresponding swap data:

(PROTOCOL _protocol, bytes memory data) = abi.decode(metadata[0], (PROTOCOL, bytes));
if (_protocol != PROTOCOL.DEX) {
revert Error.InvalidData();
}
swapProgressData.swapped = swapProgressData.swapped + _doDexSwap(data, isCollateralToIndex);
(_protocol, data) = abi.decode(metadata[1], (PROTOCOL, bytes));
if (_protocol != PROTOCOL.GMX) {
revert Error.InvalidData();
}
  • The function expects metadata to be correctly formatted, containing a valid PROTOCOL enum and associated bytes data.

  • However, there is no check ensuring that metadata actually has the expected structure before decoding.

  • If metadata[0] or metadata[1] contains invalid or malformed data, abi.decode could return incorrect values, leading to improper execution flow or unintended reverts.


Impact :

  • Reverts on Unexpected Data**: If metadata is incorrectly formatted or manipulated, the contract may revert unexpectedly, leading to failed swaps.

  • Invalid Protocol Execution: If an attacker can manipulate metadata in a way that bypasses the validation, they may cause the contract to process the wrong protocol, potentially leading to asset mismanagement.

  • Potential DOS Vector: If an external caller supplies bad data, the system may become unresponsive due to constant reverts.


Tools Used :

Manual Review

Recommendations :

To mitigate the risk associated with improper abi.decode usage, implement the following checks in a unified manner:

function _runSwap(bytes[] memory metadata, bool isCollateralToIndex, MarketPrices memory prices) internal returns (bool completed) {
if (metadata.length == 0 || metadata.length > 2) {
revert Error.InvalidData();
}
for (uint256 i = 0; i < metadata.length; i++) {
PROTOCOL _protocol;
bytes memory data;
// Use try-catch to prevent decoding failures
try this.decodeProtocol(metadata[i]) returns (PROTOCOL decodedProtocol, bytes memory decodedData) {
_protocol = decodedProtocol;
data = decodedData;
} catch {
revert Error.InvalidData();
}
// Validate protocol before processing
if (!isValidProtocol(_protocol)) {
revert Error.InvalidProtocol();
}
// Execute the appropriate swap function based on the protocol
if (_protocol == PROTOCOL.DEX) {
swapProgressData.swapped += _doDexSwap(data, isCollateralToIndex);
} else if (_protocol == PROTOCOL.GMX) {
_doGmxSwap(data, isCollateralToIndex);
} else {
revert Error.UnsupportedProtocol();
}
}
return metadata.length == 1; // Returns true only if a single protocol was used
}
// Separate function for decoding to allow try-catch usage
function decodeProtocol(bytes memory encodedData) external pure returns (PROTOCOL, bytes memory) {
return abi.decode(encodedData, (PROTOCOL, bytes));
}
// Helper function to validate protocol
function isValidProtocol(PROTOCOL _protocol) internal pure returns (bool) {
return _protocol == PROTOCOL.DEX || _protocol == PROTOCOL.GMX;
}
Updates

Lead Judging Commences

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

Suppositions

There is no real proof, concrete root cause, specific impact, or enough details in those submissions. Examples include: "It could happen" without specifying when, "If this impossible case happens," "Unexpected behavior," etc. Make a Proof of Concept (PoC) using external functions and realistic parameters. Do not test only the internal function where you think you found something.

Support

FAQs

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

Give us feedback!