Popcorn contest - 7siech's results

Lines of code

https://github.com/code-423n4/2023-01-popcorn/blob/main/src/vault/adapter/abstracts/AdapterBase.sol#L444

Vulnerability details

Impact

A malicious strategy has access to the adapter's storage and can therefore freely change any values.

Proof of Concept

Because AdapterBase calls the Strategy using delegatecall, the Strategy has access to the calling contract's storage and can be manipulated directly.

In the following proof of concept, a MaliciousStrategy is paired with the BeefyAdapter and when called will manipulate the performanceFee and highWaterMark values. Of course, any other storage slots of the adapter could also be manipulated or any other calls to external contracts on behalf of the msg.sender could be performed.

MaliciousStrategy implementation showing the exploit - https://gist.github.com/alpeware/e0b1c9f330419986142711e814bfdc7b#file-beefyadapter-t-sol-L18

Adapter helper used to determine the storage slots - https://gist.github.com/alpeware/e0b1c9f330419986142711e814bfdc7b#file-beefyadapter-t-sol-L65

BeefyAdapterTest changes made to tests -

Adding the malicious strategy - https://gist.github.com/alpeware/e0b1c9f330419986142711e814bfdc7b#file-beefyadapter-t-sol-L123

Adding new test test__StrategyHarvest() executing harvest() - https://gist.github.com/alpeware/e0b1c9f330419986142711e814bfdc7b#file-beefyadapter-t-sol-L132

Log output - https://gist.github.com/alpeware/e0b1c9f330419986142711e814bfdc7b#file-log-txt

Tools Used

Foundry

Recommended Mitigation Steps

From chatting with the devs, the goal is to mix and match adapters and strategies. I don't think delegatecall should be used and adapters and strategies should be treated as separate contracts. Relevant approvals should be given individually instead.

Lines of code

https://github.com/code-423n4/2023-01-popcorn/blob/main/src/vault/adapter/abstracts/AdapterBase.sol#L438

Vulnerability details

Impact

AdapterBase implements a cooldown period that's supposed to limit the Strategy from harvesting. However, the lastHarvest time is never updated resulting in every call to harvest() to call the underlying strategy despite there potentially being a cooldown period.

Proof of Concept

Since lastHarvest time is never updated, as soon as (lastHarvest + harvestCooldown) is > block.timestamp, any subsequent calls to harvest will call the strategy since (lastHarvest + harvestCooldown) will always be less than block.timestamp.

    function harvest() public takeFees {
        if (
            address(strategy) != address(0) &&
            ((lastHarvest + harvestCooldown) < block.timestamp)
        ) {
            // solhint-disable
            address(strategy).delegatecall(
                abi.encodeWithSignature("harvest()")
            );
        }

        emit Harvested();
    }

Tools Used

Foundry

Recommended Mitigation Steps

Update lastHarvest time -

    function harvest() public takeFees {
        if (
            address(strategy) != address(0) &&
            ((lastHarvest + harvestCooldown) < block.timestamp)
        ) {
            // *** Update last harvest time ***
            lastHarvest = block.timestamp;
            // *** Update last harvest time ***
            // solhint-disable
            address(strategy).delegatecall(
                abi.encodeWithSignature("harvest()")
            );
        }

        emit Harvested();
    }

Lines of code

https://github.com/code-423n4/2023-01-popcorn//blob/main/src/vault/Vault.sol#L88

Vulnerability details

Impact

It is possible to deploy a vault with 1e18 deposit, withdraw, management, performance fees locking up user assets.

Proof of Concept

https://gist.github.com/alpeware/bf3bf78112f3106d667c0f2dca25679e#file-vaultmaxfee-t-sol https://gist.github.com/alpeware/bf3bf78112f3106d667c0f2dca25679e#file-log-txt

Tools Used

Foundry

Recommended Mitigation Steps

In the constructor, call proposeFees(fees) [0] and changeFees() [1] instead.

[0] https://github.com/code-423n4/2023-01-popcorn//blob/main/src/vault/Vault.sol#L525 [1] https://github.com/code-423n4/2023-01-popcorn//blob/main/src/vault/Vault.sol#L540