code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-lens/blob/cdef6ebc6266c44c7068bc1c4c04e12bf0d67ead/contracts/LensHub.sol#L95

Vulnerability details

Impact

If this issue exists, _profileCounter reaches the maximum value, it will wrap around and reset to its minimum value, which is 0 for unsigned integers in Solidity. This unintended behavior can have several negative impacts on the functioning of the contract like; As _profileCounter resets to 0, it will start assigning duplicate profile IDs to new profiles created after the overflow occurs. This could lead to conflicts, incorrect data associations, and potential errors when interacting with profiles.

Proof of Concept

// SPDX-License-Identifier: MIT pragma solidity ^0.8.15;

contract ProfileCounterBugPOC { uint256 public _profileCounter;

constructor() {
    _profileCounter = 0;
}

function createProfile() public {
    // The unchecked increment will lead to an overflow when _profileCounter reaches its maximum value
    unchecked {
        _profileCounter++;
    }
}

function getCurrentProfileCounter() public view returns (uint256) {
    return _profileCounter;
}

}

we have a ProfileCounterBugPOC contract with an _profileCounter variable initialized to 0. The createProfile function increments _profileCounter using unchecked ++_profileCounter. This operation may lead to an overflow when _profileCounter reaches its maximum value.

Let's see what happens when we call the createProfile function repeatedly until _profileCounter overflows,

ProfileCounterBugPOC contract = new ProfileCounterBugPOC(); uint256 maxUint256 = type(uint256).max;

// Increment _profileCounter until it overflows for (uint256 i = 0; i <= maxUint256; i++) { contract.createProfile(); }

// Get the value of _profileCounter after the overflow uint256 currentProfileCounter = contract.getCurrentProfileCounter();

Hence currentProfileCounter become 0, indicating that _profileCounter has wrapped around to its minimum value after the overflow.

Tools Used

Recommended Mitigation Steps

Replacing the unchecked increment operation (unchecked ++_profileCounter) with a safe addition operation using the SafeMath library will be helpful. The SafeMath library prevents arithmetic overflows and underflows in integer operations. Or Before incrementing _profileCounter, add a check to ensure that it has not reached its maximum value (type(uint256).max).