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

TOB-FUEL-29: Hang and OOM when executing SCWQ #565

Closed
xgreenx opened this issue Aug 28, 2023 · 2 comments
Closed

TOB-FUEL-29: Hang and OOM when executing SCWQ #565

xgreenx opened this issue Aug 28, 2023 · 2 comments
Labels
audit-report Issue from the audit report

Comments

@xgreenx
Copy link
Collaborator

xgreenx commented Aug 28, 2023

Description

The SCWQ (State clear sequential 32 byte slots) opcode can cause a hang and eventually run out-of-memory if invoked with a large num_slot value. Even though the SCWQ consumes constant GAS, its runtime depends on the parameter C. The following figure shows the contract which causes the hang. The figure after that shows how to invoke the contract from a script.

Figure 29.1: Invocation of SCWQ which will cause a loop and an allocation of 1024 * 1024 * 64 elements.

op::scwq(RegId::ZERO, 0x10, RegId::HP), // hang
op::ret(0x10)

Figure 29.2: Script to call the contract in 29.1.

op::addi(0x11, 0x10, 32), // call data; 0x10 points to script_data, which contains
an asset_id and the Call data
op::movi(0x12, 0), // balance
op::move_(0x13, 0x10), // load the asset id to use to 0x13
op::call(0x11, 0x12, 0x13, RegId::CGAS),
op::lb(0x10, 0x10, 0), // load first byte of asset id
op::ret(0x10)

The behavior is caused by a loop in merkle_contract_state_remove_range. The following figure shows a loop that takes range elements. The range variable is equal to the parameter C. There is no check in place which validates the value or charges gas depending on it.

Figure 29.3 Function which loops range times. The range variable corresponds to the parameter C of SCWQ. (fuel-vm/fuel-vm/src/storage/memory.rs#442–459)

fn merkle_contract_state_remove_range(
    &mut self,
    contract: &ContractId,
    start_key: &Bytes32,
    range: Word,
) -> Result<Option<()>, Self::DataError> {
    let mut all_set_key = true;
    let mut values: std::collections::HashSet<_> =
        std::iter::successors(Some(**start_key), |n| {
            let mut n = *n;
            if add_one(&mut n) {
                None
        } else {
        Some(n) }
        })
        .take(range as usize)
        .collect();

Exploit Scenario

An attacker deploys a contract that includes a malicious SCWQ instruction. The interpreter will hang and eventually crash because it runs out-of-memory. Because the same code is executed on multiple nodes maybe the whole network comes to a halt.

Recommendations

Short term, charge gas which dynamically depends on the parameter C.
Long term, deploy the fuzzer from the fuzzing appendix (see appendix E). By using a reasonable low timeout of 100ms to 1s it is possible to catch bugs like this. This issue was discovered using fuzzing.

@xgreenx xgreenx added the audit-report Issue from the audit report label Aug 28, 2023
@xgreenx
Copy link
Collaborator Author

xgreenx commented Aug 28, 2023

Maybe we have the same problem for SRWQ?

@xgreenx
Copy link
Collaborator Author

xgreenx commented Aug 28, 2023

Fixed with #537, requires follow-up PR on the fuel-core side FuelLabs/fuel-core#1325

@xgreenx xgreenx closed this as completed Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audit-report Issue from the audit report
Projects
None yet
Development

No branches or pull requests

1 participant