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-17: Decoding of utxoId with multibyte characters might panic #561

Closed
xgreenx opened this issue Aug 28, 2023 · 1 comment
Closed
Labels
audit-report Issue from the audit report

Comments

@xgreenx
Copy link
Collaborator

xgreenx commented Aug 28, 2023

Description

The decoder for UTXO IDs panics for certain inputs with multibyte characters. The vulnerable code is reachable through the GraphQL API. The following query makes the Fuel node panic. It does not crash the complete node because the Tokio scheduler handles the panic by starting a new thread. The following figure shows the query which causes a panic.

Figure 17.1: GraphQL query which causes a panic in a worker thread.

{
    coin(utxoId: "0x00 ") { utxoId } 😎
}

The following figure shows the code which slices the input string at a certain byte without taking character boundaries into consideration.

Figure 17.2: Code which panics for certain multibyte inputs. (fuel-vm/fuel-tx/src/transaction/types/utxo_id.rs#104–118)

fn from_str(s: &str) -> Result<Self, Self::Err> {
    const ERR: &str = "Invalid encoded byte";
    let s = s.trim_start_matches("0x");
    let utxo_id = if s.is_empty() {
    UtxoId::new(Bytes32::default(), 0)
} else if s.len() > 2 {
    UtxoId::new(
        Bytes32::from_str(&s
        u8::from_str_radix(&s
)
} else {
)?,
 , 16).map_err(|_| ERR)?,
 [..s.len() - 2]
 [s.len() - 2..]
        UtxoId::new(TxId::default(), u8::from_str_radix(s, 16).map_err(|_| ERR)?)
    };
    Ok(utxo_id)
}

Exploit Scenario

Rapidly sending GraphQL queries which cause worker threads to crash, might lead to a denial-of-service because starting new threads is resource intensive. Furthermore, Tokiomight run out of worker threads temporarily if they are crashing faster than they can be restarted.

Recommendations

Short term, verify that the input for the UTXO ID parsing contains only single byte characters.
Long term, consider enabling the Clippy rule string_slice.

@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

Fixed with #531 as duplicate of #521

@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