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

12 stars 7 forks source link

Unchecked Deposits Enabling Share Price Manipulation #170

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L407-L415

Vulnerability details

Impact

The deposit mechanism in the provided code snippet lacks proper checks and validations to ensure that users have enough assets to deposit. This vulnerability can be exploited by malicious actors to manipulate share prices, create imbalances, and exploit yield generation mechanisms within the protocol.

Proof of Concept

The deposit function in the provided code snippet lacks proper checks to ensure users have enough assets to deposit. This can be exploited by malicious actors to manipulate share prices, create imbalances, and exploit yield generation mechanisms within the protocol. The following code snippet demonstrates a possible attack scenario:

// Attacker contract
contract MaliciousContract {
    Vault vault; // Assume the contract has a reference to the Vault contract

    function attack() public {
        // Deposit non-existent or insufficient assets repeatedly
        while (true) {
            uint256 assetsToDeposit = 1; // Arbitrary value, assumed to be less than what the attacker actually possesses

            // Call the vulnerable deposit function
            vault.deposit(assetsToDeposit, address(this));
        }
    }
}

By repeatedly attempting to deposit non-existent or insufficient assets, an attacker can artificially inflate or deflate the total assets held by the protocol, leading to unfair advantages and potential financial gains.

Tools Used

manual

Recommended Mitigation Steps

The function should compare the user's asset balance with the amount they intend to deposit. If the user's asset balance is less than the intended deposit amount, the function should revert the transaction, preventing the deposit from proceeding.

Assessed type

Other

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Invalid