code-423n4 / 2023-07-lens-findings

0 stars 0 forks source link

Incorrect number conversion #156

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-lens/blob/main/contracts/FollowNFT.sol#L387 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/FollowNFT.sol#L392

Vulnerability details

Impact

The _baseFollow() function receives the parameter uint256 followerProfileId (https://github.com/code-423n4/2023-07-lens/blob/main/contracts/FollowNFT.sol#L387).

Later (https://github.com/code-423n4/2023-07-lens/blob/main/contracts/FollowNFT.sol#L392) the parameter is written to _followDataByFollowTokenId[followTokenId].followerProfileId:

_followDataByFollowTokenId[followTokenId].followerProfileId = uint160(followerProfileId).

However, here it is converted to uint160(followerProfileId). Thus, if followerProfileId > type(uint160).max, the followerProfileId parameter will be converted incorrectly.

Tools Used

VSCode

Recommended Mitigation Steps

Everywhere change followerProfileId to either uint256 or uint160.

Assessed type

Under/Overflow

141345 commented 1 year ago

QA might be more appropriate.

donosonaumczuk commented 1 year ago

It is ok as it is, we are assuming Profile ID will be created sequentially, thus it should never be greater than type(uint160).max. The reason why we kept using 256 bits it's just because we had it like that in Lens V1, and we just cast it in the places that we need it. In this case, in the Follow NFT was done for gas optimization (struct storage packing).

c4-sponsor commented 1 year ago

donosonaumczuk marked the issue as sponsor disputed

c4-judge commented 1 year ago

Picodes changed the severity to QA (Quality Assurance)

Picodes commented 1 year ago

Assuming increments are sequential it's fine to assume that this will never overflow (2^160 > 10^48)

c4-judge commented 1 year ago

Picodes marked the issue as grade-c