Unchecked Resource Consumption issue in Storage of Wasm Code

[c4-bot-4]

Lines of code

https://github.com/code-423n4/2024-03-phala-network/blob/a01ffbe992560d8d0f17deadfb9b9a2bed38377e/phala-blockchain/crates/pink/runtime/src/runtime/pallet_pink.rs#L134-L146

Vulnerability details

Vulnerability details

The put_sidevm_code function within the specified pallet allows for the storage of wasm bytecode associated with a given owner account. and this function computes a fee based on the size of the bytecode plus a static item deposit fee. However, there is no explicit check or limit on the maximum size of the bytecode vector (Vec) that can be stored. Consequently, this oversight facilitates a potential attack vector where a malicious user could upload excessively large wasm binaries, leading to unchecked resource consumption. Such actions could significantly degrade blockchain performance due to storage exhaustion, increase operational costs for node operators, and potentially lead to a denial-of-service condition.

pub fn put_sidevm_code(
    owner: T::AccountId,
    code: Vec<u8>,
) -> Result<T::Hash, DispatchError> {
    let hash = T::Hashing::hash(&code);
    let bytes = code.len() + hash.as_ref().len();
    let fee = Self::deposit_per_byte()
        .saturating_mul(BalanceOf::<T>::saturated_from(bytes))
        .saturating_add(Self::deposit_per_item());
    Self::pay(&owner, fee)?;
    <SidevmCodes<T>>::insert(hash, WasmCode { owner, code });
    Ok(hash)
}

Impact

A malicious actor could exploit this vulnerability by repeatedly uploading very large wasm binaries to the blockchain. Since there's no explicit limit on the code size that can be stored, these actions could lead to significant storage consumption on nodes participating in the blockchain network.

Proof of Concept

class MockBlockchain:
    def __init__(self):
        self.storage = {}
        self.fee_schedule = {'deposit_per_byte': 1, 'deposit_per_item': 100}
        self.total_fees_collected = 0

    def put_sidevm_code(self, owner, code):
        """
        Simulates storing wasm code in the blockchain storage.
        """
        code_size = len(code)
        fee = self.calculate_fee(code_size)

        # Simulate fee payment (here, simply adding to a total)
        self.total_fees_collected += fee

        # Store the code if fee payment is successful
        self.storage[owner] = code

        print(f"Stored code of size {code_size} bytes for owner '{owner}' with fee {fee}")

    def calculate_fee(self, code_size):
        """
        Calculates the fee based on the size of the code.
        """
        return self.fee_schedule['deposit_per_byte'] * code_size + self.fee_schedule['deposit_per_item']

# Create a mock blockchain instance
mock_blockchain = MockBlockchain()

# Define a list of code sizes to test, illustrating the lack of a size check
code_sizes = [10, 100, 1000, 10000, 100000, 1000000, 10000000]

for size in code_sizes:
    # Generate dummy code of specified size
    dummy_code = b'a' * size
    mock_blockchain.put_sidevm_code("Alice", dummy_code)

print(f"Total fees collected: {mock_blockchain.total_fees_collected}")

as result : the Codes of varying sizes, ranging from 10 bytes to 10,000,000 bytes, were stored successfully for a single owner, 'Alice'. With each increase in code size, the fee calculated and collected also increased proportionally, based on the simplistic fee calculation model (1 unit per byte of code plus a flat fee of 100 units). The test concluded with a total of 11,111,810 units collected in fees, reflecting the cumulative cost of storing all the provided codes.

Tools Used

manual review

Recommended Mitigation Steps

need a checks on the size of the wasm bytecode being uploaded through the put_sidevm_code function. Establish a maximum allowable size for wasm binaries that balances the need for legitimate functionality with the prevention of abuse

Assessed type

Other

[c4-pre-sort]

141345 marked the issue as insufficient quality report

[141345]

wasm bytecode size invalid, the attacker pays the fee

[c4-sponsor]

kvinwang (sponsor) acknowledged

[kvinwang]

wasm bytecode size invalid, the attacker pays the fee

Correct. And there is a size limit check on-chain. But an additional check in the pink-runtime is a nice to have feature. Better as a QA level.

[c4-sponsor]

kvinwang (sponsor) confirmed

[c4-sponsor]

kvinwang marked the issue as disagree with severity

[c4-judge]

OpenCoreCH changed the severity to QA (Quality Assurance)

[c4-judge]

OpenCoreCH marked the issue as grade-b