code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-02-aave-lens/blob/main/contracts/core/modules/follow/SecretCodeFollowModule.sol#L14-L23

Vulnerability details

Impact

Users can set followModule to SecretCodeFollowModule through the setFollowModule function to make other users have to provide passcode before they can become followers.

When the initializeFollowModule method of SecretCodeFollowModule is called, the plaintext of the passcode will be returned, and the plaintext of the passcode will be sent as the event parameter in the Events.FollowModuleSet event.

Also, setting the passcode in clear text can be easily cracked.

    function initializeFollowModule(uint256 profileId, bytes calldata data)
        external
        override
        onlyHub
        returns (bytes memory)
    {
        uint256 passcode = abi.decode(data, (uint256));
        _passcodeByProfile[profileId] = passcode;
        return data;
    }
...
    function setFollowModule(
        uint256 profileId,
        address followModule,
        bytes calldata followModuleData,
        DataTypes.ProfileStruct storage _profile,
        mapping(address => bool) storage _followModuleWhitelisted
    ) external {
        address prevFollowModule = _profile.followModule;
        if (followModule != prevFollowModule) {
            _profile.followModule = followModule;
        }

        bytes memory followModuleReturnData = _initFollowModule(
            profileId,
            followModule,
            followModuleData,
            _followModuleWhitelisted
        );
        emit Events.FollowModuleSet(
            profileId,
            followModule,
            followModuleReturnData,
            block.timestamp
        );
    }

Proof of Concept

https://github.com/code-423n4/2022-02-aave-lens/blob/main/contracts/core/modules/follow/SecretCodeFollowModule.sol#L14-L23

https://github.com/code-423n4/2022-02-aave-lens/blob/main/contracts/libraries/PublishingLogic.sol#L80-L104

Tools Used

None

Recommended Mitigation Steps

Considering that the passcode should be kept as secret as possible, the data passed in by the user in the initializeFollowModule function should be the passcode processed by keccak256, and the passcode provided by the user in the processFollow function needs to be processed by keccak256 and then compared with _passcodeByProfile[profileId].

      mapping(uint256 => bytes) internal _passcodeByProfile;

     // data should be passcode processed by keccak256
     function initializeFollowModule(uint256 profileId, bytes calldata data)
         external
         override
         onlyHub
         returns (bytes memory)
     {
         _passcodeByProfile[profileId] = data;
         return data;
     }

     function processFollow(
         address follower,
         uint256 profileId,
         bytes calldata data
     ) external view override {
         uint256 passcode = abi.decode(data, (uint256));
         if (keccak256(abi.encodePacked(passcode)) != _passcodeByProfile[profileId]) revert PasscodeInvalid();
     }

oneski commented 2 years ago

per comments in the discord this contract is out of scope

tabshaikh commented 2 years ago

duplicate of #11 by the same warden

0xleastwood commented 2 years ago

As per the contest README, this asset is out of scope.

code-423n4 / 2022-02-aave-lens-findings