Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: medium
Invalid

The contractInteractions function overwrites previous interaction data for the same target

Description

The contractInteractions() function in the InheritanceManager contract allows the owner to interact with external contracts and optionally store the result of these interactions. However, a vulnerability exists where the function unconditionally overwrites any previous interaction data when storing the result of a new interaction with the same target address.

function contractInteractions(address _target, bytes calldata _payload, uint256 _value, bool _storeTarget)
external
nonReentrant
onlyOwner
{
(bool success, bytes memory data) = _target.call{value: _value}(_payload);
require(success, "interaction failed");
if (_storeTarget) {
interactions[_target] = data;
}
}

This vulnerability directly contradicts the function's documented intention to "make it clear to beneficiaries where to look for funds outside this contract." Since each new interaction with the same target overwrites the previous data, beneficiaries will only have visibility into the most recent interaction, losing all history of prior interactions.

Impact

  1. Loss of transaction history: The contract documentation explicitly states that the interactions mapping is meant to store transaction history for beneficiaries to track funds outside the contract. This overwriting behavior renders that functionality useless, as only the most recent interaction is preserved.

  2. Asset traceability issues: In case of inheritance, beneficiaries will lose the ability to trace the owner's previous interactions with external protocols, potentially making it impossible to locate and recover assets deposited in DeFi platforms or other contracts.

  3. Integrity compromise: The system fails to maintain a complete record of contract interactions as intended, compromising the integrity of the inheritance management system.

Proof of Concept (PoC)

The following test demonstrates how the second interaction with the same target address completely overwrites the data from the first interaction:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
import "forge-std/Test.sol";
import "../src/InheritanceManager.sol";
// Simple contract that simulates deposits on a DeFi platform
contract DeFiPlatform {
mapping(address => uint256) public deposits;
event Deposit(address indexed user, uint256 amount);
event Withdrawal(address indexed user, uint256 amount);
function deposit(uint256 _amount) external returns (bytes memory) {
deposits[msg.sender] += _amount;
emit Deposit(msg.sender, _amount);
return abi.encode("deposit", _amount);
}
function withdraw(uint256 _amount) external returns (bytes memory) {
require(deposits[msg.sender] >= _amount, "Insufficient balance");
deposits[msg.sender] -= _amount;
emit Withdrawal(msg.sender, _amount);
return abi.encode("withdraw", _amount);
}
}
// Helper contract to expose interactions mapping
contract ExposedInheritanceManager is InheritanceManager {
function getInteraction(address _target) external view returns (bytes memory) {
return interactions[_target];
}
}
contract PoC is Test {
ExposedInheritanceManager inheritanceManager;
DeFiPlatform defiPlatform;
address owner;
function setUp() public {
inheritanceManager = new ExposedInheritanceManager();
defiPlatform = new DeFiPlatform();
owner = inheritanceManager.getOwner();
vm.startPrank(owner);
}
function test_InteractionsValueOverwrite() public {
// First interaction - Deposit funds
bytes memory depositPayload = abi.encodeWithSignature("deposit(uint256)", 1000);
inheritanceManager.contractInteractions(address(defiPlatform), depositPayload, 0, true);
// Show interactions[_target] after first deposit
bytes memory firstInteraction = inheritanceManager.getInteraction(address(defiPlatform));
(string memory action1, uint256 amount1) = abi.decode(firstInteraction, (string, uint256));
console.log("interactions[_target] after first deposit:", action1, amount1);
// Second interaction - Partial withdrawal
bytes memory withdrawPayload = abi.encodeWithSignature("withdraw(uint256)", 400);
inheritanceManager.contractInteractions(address(defiPlatform), withdrawPayload, 0, true);
// Show interactions[_target] after withdraw
bytes memory secondInteraction = inheritanceManager.getInteraction(address(defiPlatform));
(string memory action2, uint256 amount2) = abi.decode(secondInteraction, (string, uint256));
console.log("interactions[_target] after withdraw:", action2, amount2);
// Assert that the second interaction has completely overwritten the first
assertTrue(
keccak256(firstInteraction) != keccak256(secondInteraction),
"The first interaction was overwritten by the second interaction"
);
vm.stopPrank();
console.log("VULNERABILITY CONFIRMED: The first interaction was overwritten by the second interaction");
}
}

Place the test in the test folder and run it with the following command:

forge test --match-test test_InteractionsValueOverwrite -vv

Practical Impact Example

Consider this real-world scenario:

  1. The contract owner deposits 10,000 USDC into Aave lending protocol

  2. Later, the owner withdraws 2,000 USDC from Aave

  3. If the owner becomes inactive for 90+ days, beneficiaries would only see the withdrawal record of 2,000 USDC

  4. Beneficiaries would have no knowledge of the remaining 8,000 USDC deposit in Aave, potentially leading to permanent loss of those funds

Recommendations

Consider the following approach to fix this issue:

Use an array to store interaction history:

// Change the state variable
mapping(address protocol => bytes[]) interactions;
// Modify the relevant part of contractInteractions function
if (_storeTarget) {
interactions[_target].push(data);
}

By implementing this solution, the contract would maintain a complete history of interactions with external protocols, ensuring beneficiaries have full visibility into where to locate assets outside of the contract as intended.

Updates

Lead Judging Commences

0xtimefliez Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Appeal created

jomar Submitter
3 months ago
0xtimefliez Lead Judge
3 months ago
0xtimefliez Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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