c4-bot-3 commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-03-phala-network/blob/a01ffbe992560d8d0f17deadfb9b9a2bed38377e/phala-blockchain/crates/pink/runtime/src/contract.rs#L75-L84 https://github.com/code-423n4/2024-03-phala-network/blob/a01ffbe992560d8d0f17deadfb9b9a2bed38377e/phala-blockchain/crates/pink/runtime/src/contract.rs#L83

Vulnerability details

MED: Unhandled Err variant in mask_low_bits128 (crates/pink/runtime/src/contract.rs:56-77)

Description

The mask_low_bits128 macro is used to mask the low bits of a u128 value. However, the macro does not handle the case when the min_mask_bits is greater than 128, which could lead to unexpected behavior or runtime errors.

define_mask_fn!(mask_low_bits128, 128, u128);

fn mask_deposit(deposit: u128, deposit_per_byte: u128) -> u128 {
    const MIN_MASKED_BYTES: u128 = 256;
    let min_masked_value = deposit_per_byte
        .saturating_mul(MIN_MASKED_BYTES)
        .saturating_sub(1);
    let min_mask_bits = 128 - min_masked_value.leading_zeros();
    mask_low_bits128(deposit, min_mask_bits)
}

The fact that the mask_low_bits128 macro does not handle the case when the min_mask_bits argument is greater than 128. The specific edge case that can lead to this vulnerability is when the min_masked_value is very large, resulting in a min_mask_bits value that exceeds 128.

The line responsible Is: https://github.com/code-423n4/2024-03-phala-network/blob/a01ffbe992560d8d0f17deadfb9b9a2bed38377e/phala-blockchain/crates/pink/runtime/src/contract.rs#L83

mask_low_bits128(deposit, min_mask_bits)

If min_mask_bits is greater than 128, the behavior of the mask_low_bits128 macro is undefined and could lead to unexpected results or runtime errors.

Under normal circumstances, the mask_low_bits128 macro is expected to mask the low bits of the provided u128 value based on the min_mask_bits argument. The macro should handle the case when min_mask_bits is within the valid range of 0 to 128 and return the masked value accordingly.

Due to the lack of proper handling for the case when min_mask_bits is greater than 128, the actual behavior of the mask_low_bits128 macro is undefined. It may produce incorrect results, trigger runtime errors, or even cause the Pink Runtime to crash.

Impact

If the macro is called with a min_mask_bits value greater than 128, it can lead to incorrect masking of the deposit value in the mask_deposit function. This could result in incorrect calculations, inconsistencies in the runtime's behavior, or even runtime failures.

Tools Used

Manual audit

Recommended Mitigation Steps

Modify the mask_low_bits128 macro to properly handle the case when min_mask_bits is greater than 128. Consider adding a check to ensure that min_mask_bits is within the valid range (0 to 128) and return an error or use a default value if it exceeds the range.