popeye - Unstable Transaction Handling in `TelcoinDistributor::proposeTransaction` due to mismatch in array lengths

[sherlock-admin]

popeye

high

Unstable Transaction Handling in TelcoinDistributor::proposeTransaction due to mismatch in array lengths

Summary

Vulnerability Detail

In the TelcoinDistributor::proposeTransaction function, users (specifically council members) can propose transactions by providing two arrays: destinations (recipient addresses) and amounts (Telcoin amounts). The absence of a check to ensure these arrays are of equal length allows for the possibility of submitting a proposal with an unequal number of destinations and amounts. This design flaw is a significant oversight in the contract's logic.

Impact

It can lead to unpredictable transaction behavior, potential financial loss, and a compromise in the contract’s integrity.

Proof of Concept

Consider a scenario with Alice and Attacker:

• Alice (Honest Council Member): Intends to propose a legitimate transaction.
• Attacker (Malicious Council Member): Seeks to exploit the array length mismatch vulnerability.

Step-by-Step Exploitation

1. Legitimate Proposal by Alice:

   • Alice prepares a transaction proposal to distribute 400 Telcoin equally among four recipients.
   • She correctly sets destinations: [Address1, Address2, Address3, Address4].
   • She correctly sets amounts: [100, 100, 100, 100].
   • Alice submits the proposal using proposeTransaction.

2. Malicious Proposal by Attacker:

   • The attacker, another council member, prepares a transaction proposal.
   • The attacker deliberately sets mismatched arrays:
     • destinations: [VictimAddress1, VictimAddress2].
     • amounts: [150, 150, 200] (Intentionally providing an extra amount).
   • The attacker submits this proposal using proposeTransaction.

3. Contract's Flawed Acceptance:

   • The contract, lacking a validation check, accepts both proposals.
   • The attacker's proposal now has an unallocated amount of 200 Telcoin.

4. Execution of Proposals:

   • When proposals are executed:
     • Alice’s proposal is processed normally.
     • In the attacker's proposal, the first two amounts (150 Telcoin each) are sent to the two specified victims.
     • The extra 200 Telcoin remains unallocated, creating a discrepancy in the contract's ledger.

5. Consequences:

   • The unallocated 200 Telcoin results in an inconsistent state within the contract.

Code Snippet

https://github.com/sherlock-audit/2024-01-telcoin/blob/main/telcoin-audit/contracts/protocol/core/TelcoinDistributor.sol#L87-L106

Tool used

Manual Review

Recommendation

Implement an array length equality check in the proposeTransaction function:

function proposeTransaction(
    uint256 totalWithdrawl,
    address[] memory destinations,
    uint256[] memory amounts
) external onlyCouncilMember whenNotPaused {
    require(destinations.length == amounts.length, "Array length mismatch: destinations and amounts must be equal.");

    // Rest of the existing function code
    // ...
}

Duplicate of #2

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid because { this is invalid because the contract ensures that all the amounts specified have been sent to the recipinet and if that didnt happen the function will revert with leftover issues}