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

Sending a transaction in web-client may not always unsubscribe from address #2456

Closed
jsdanielh opened this issue May 9, 2024 · 0 comments · Fixed by #2480
Closed

Sending a transaction in web-client may not always unsubscribe from address #2456

jsdanielh opened this issue May 9, 2024 · 0 comments · Fixed by #2480
Assignees

Comments

@jsdanielh
Copy link
Contributor

Link: https://hackerone.com/reports/2491408
Date: 2024-05-06 06:08:42 UTC
By: ryanrb
Weakness: Improper Resource Shutdown or Release

Details:

Summary

In web-client, the method send_transaction will conditionally subscribe to an address based on specific conditions. Later in the method, the code ensures that this is cleaned up by unsubscribing from the address.

However, the actual method for sending a transaction uses the ? operator, which means the operation can fail and return at that point—potentially causing the method to return after subscribing but before cleaning up the subscription.

consensus.send_transaction is invoked with ?

Project: core-rs-albatross
File reference: web-client/src/client/lib.rs
Line: 555

// Actually send the transaction
consensus.send_transaction(tx.native()).await?;

The address is not unsubscribed from until line 574

Project: core-rs-albatross
File reference: web-client/src/client/lib.rs
Line: 578

// Unsubscribe from any address we subscribed to, without caring about the result
if let Some(address) = subscribed_address {
    let owned_consensus = consensus.clone();
    spawn_local(async move {
        let _ = owned_consensus
            .unsubscribe_from_addresses(vec![address], 1)
            .await;
    });
}

Recommendation

Ensure that the address is unsubscribed from regardless of the success or failure of the outcome of send_transaction.

References:

Impact

Depending on the amount of resources that subscribing to an address takes up, enough failed transactions could lead to resource exhaustion over time if nothing else cleans up the subscription.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants