Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Receipt size can increase after the size has been validated #12606

Open
jancionear opened this issue Dec 11, 2024 · 0 comments
Open

Receipt size can increase after the size has been validated #12606

jancionear opened this issue Dec 11, 2024 · 0 comments
Assignees
Labels
C-bug Category: This is a bug

Comments

@jancionear
Copy link
Contributor

Description

There is a limit on maximum receipt size, it should ensure that all receipts have size below max_receipt_size (currently 4MiB). It was added in #11492.

The limit is implemented as a check in validate_receipt:

/// Validates a given receipt. Checks validity of the Action or Data receipt.
pub(crate) fn validate_receipt(
    limit_config: &LimitConfig,
    receipt: &Receipt,
    current_protocol_version: ProtocolVersion,
) -> Result<(), ReceiptValidationError> {
    let receipt_size: u64 =
        borsh::to_vec(receipt).unwrap().len().try_into().expect("Can't convert usize to u64");
    if receipt_size > limit_config.max_receipt_size {
        return Err(ReceiptValidationError::ReceiptSizeExceeded {
            size: receipt_size,
            limit: limit_config.max_receipt_size,
        });
    }

validate_receipt is run for every new receipt that is created and receipts that are too large are rejected:

if new_result.result.is_ok() {
    if let Err(e) = new_result.new_receipts.iter().try_for_each(|receipt| {
        validate_receipt(
            &apply_state.config.wasm_config.limit_config,
            receipt,
            apply_state.current_protocol_version,
        )
    }) {
        new_result.result = Err(ActionErrorKind::NewReceiptValidationError(e).into());
    }
}

But there is a problem - a new receipt can be modified after the validation is done.
For example, new output_data_receivers can be added:

// Generating outgoing data
// A {
// B().then(C())}  B--data receipt->C

// A {
// B(); 42}
if !action_receipt.output_data_receivers.is_empty() {
    if let Ok(ReturnData::ReceiptIndex(receipt_index)) = result.result {
        // Modifying a new receipt instead of sending data
        match result
            .new_receipts
            .get_mut(receipt_index as usize)
            .expect("the receipt for the given receipt index should exist")
            .receipt_mut()
        {
            ReceiptEnum::Action(ref mut new_action_receipt)
            | ReceiptEnum::PromiseYield(ref mut new_action_receipt) => new_action_receipt
                .output_data_receivers
                .extend_from_slice(&action_receipt.output_data_receivers),
            _ => unreachable!("the receipt should be an action receipt"),
        }
    }

It's possible to create a receipt which passes validate_receipt, but then adding output_data_receivers causes it to go above the size limit. Later the modified receipt is validated again (as an incoming receipt) and the validation fails, which causes runtime to panic. The panic makes sense, as we don't expect invalid receipts in the incoming receipts set, the validation check is a sanity check. The problem is that the receipt was allowed to go above the max allowed size.

This issue is already mitigated on 2.3.1 (60687b8) and 2.4.0 (1b2565b). I'll make a PR to master soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug
Projects
None yet
Development

No branches or pull requests

1 participant