Inheritable Smart Contract Wallet
First Flight #35
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: high
Invalid

The current implementation allows data collisions in the interactions mapping, potentially leading overwritten contract interaction data or losing funds if interaction is forgotten.

alekseyts

Summary

A high severity vulnerability has been identified in the InheritanceManager contract's storage management for contract interactions. The current implementation allows data collisions in the interactions mapping, potentially leading overwritten contract interaction data or losing funds if interaction is forgotten.

Vulnerability Details

The vulnerability exists in the storage structure for contract interactions:

  1. Current Implementation Issue:

  • Uses a simple mapping: mapping(address protocol => bytes) public interactions

  • Each protocol address can only store one interaction

  • New interactions override previous ones for the same protocol address

  1. Test Demonstration:

  • Two different interactions are created for the same contract:

    • "create short order call1"

    • "stake funds call2"

  • When both are stored, only the latest interaction data remains

  • The test confirms this by showing the second interaction data overwrites the first

From the test:

//SPDX-License-Identifier: MIT
​
pragma solidity 0.8.26;
​
import {Test, console} from "forge-std/Test.sol";
import {InheritanceManager} from "../src/InheritanceManager.sol";
import {ERC20Mock} from "@openzeppelin/contracts/mocks/token/ERC20Mock.sol";
​
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
​
contract InheritanceManagerAuditTest is Test {
InheritanceManager im;
ERC20Mock usdc;
ERC20Mock weth;
​
address owner = makeAddr("owner");
address user1 = makeAddr("user1");
​
function setUp() public {
vm.prank(owner);
im = new InheritanceManager();
usdc = new ERC20Mock();
weth = new ERC20Mock();
}
​
// test contractInteractions for and overriden data if we interact with same contract every time
// fix: it could be used another data structure to keep all interactions like: InheritanceManager::mapping(address protocol => mapping(uint256 interactionId => bytes) public interactions;
// NOTE: for test purpose I set public access modifier to: InheritanceManager::mapping(address protocol => bytes) public interactions;
function test_contractInteractions_OverrideInteraction() public {
address someContract1 = makeAddr("someContract1");
​
uint256 amount = 10e10;
vm.deal(address(im), amount);
​
vm.startPrank(owner);
bytes4 selector = bytes4(keccak256("someFunction(string)"));
bytes memory bytesAsVar1 = abi.encodeWithSelector(
selector,
"create short order call1"
);
bytes memory bytesAsVar2 = abi.encodeWithSelector(
selector,
"stake funds call2"
);
​
vm.mockCall(
someContract1,
abi.encodeWithSelector(bytes4(keccak256("someFunction(string)"))), // matches any function selector
abi.encode(bytesAsVar1)
);
im.contractInteractions(someContract1, bytesAsVar1, 10, true);
vm.clearMockedCalls();
​
vm.mockCall(
someContract1,
abi.encodeWithSelector(bytes4(keccak256("someFunction(string)"))),
abi.encode(bytesAsVar2)
);
im.contractInteractions(someContract1, bytesAsVar2, 10, true);
vm.clearMockedCalls();
​
vm.stopPrank();
​
bool hasSubstr = contains(bytesAsVar2, im.interactions(someContract1));
assert(hasSubstr);
​
// bytes32 keySlot = bytes32(keccak256(abi.encode(someContract1, 5)));
// bytes32 value1 = bytes32(vm.load(address(im), keySlot));
}
​
function contains(
bytes memory whatBytes,
bytes memory whereBytes
) internal pure returns (bool) {
​
require(whereBytes.length >= whatBytes.length);
​
bool found = false;
for (uint i = 0; i <= whereBytes.length - whatBytes.length; i++) {
bool flag = true;
for (uint j = 0; j < whatBytes.length; j++)
if (whereBytes[i + j] != whatBytes[j]) {
flag = false;
break;
}
if (flag) {
found = true;
break;
}
}
​
return found;
}
}
​

Impact

High severity. The vulnerability allows:

  • Loss of funds based on loss of historical interaction data

  • Loss of historical interaction data

  • Potential disruption of protocol interaction sequences

  • Inability to maintain multiple different interactions with the same protocol

Tools Used

  • Manual code review

  • Foundry test framework

  • Mock contract calls for testing

  • Storage layout analysis

Recommendations

  1. Implement nested mapping structure:

mapping(address protocol => mapping(uint256 interactionId => bytes)) public interactions;

  1. Add interaction ID management:

  • Implement counter per protocol address

  • Allow explicit interaction ID specification

  • Add methods to query available interactions

  1. Enhance data structure with additional features:

  • Add timestamp for interactions

  • Include status flags (active/inactive)

  • Implement interaction deletion capability

Example fix structure:

contract InheritanceManager {
// Counter for interaction IDs
mapping(address => uint256) private _interactionCounter;
// Enhanced storage structure
mapping(address => mapping(uint256 => bytes)) public interactions;
function contractInteractions(
address protocol,
bytes memory data,
uint256 executionDelay,
bool active
) external {
uint256 interactionId = _interactionCounter[protocol]++;
interactions[protocol][interactionId] = data;
// Additional implementation...
}
}
Updates

Lead Judging Commences

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

Support

FAQs

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

Give us feedback!