Do not panic when decrypting history file

Impact

Unhandled panics can be leveraged by an attacker to cause the application to crash, thereby achieving a denial-of-service attack. This is particularly true when parsing data from an untrusted source where the attacker can influence inputs directly.

Description

This specific instance of an unhandled panic is highlighted because it may be triggered by untrusted input from the filesystem. A complete list of potential panics within the reviewed code was not completed.

The function decrypt_history_file(), as its name implies, is used to decrypt a message history file encrypted using AES-GCM, with the usual ciphertext consisting of the concatenation of a nonce, ciphertext, and authentication tag. The code parses the nonce as follows:

// Read the messages file content
let mut input_file = File::open(input_path)?;
let mut buffer = Vec::new();
input_file.read_to_end(&mut buffer)?;

// Split the nonce and ciphertext
let (nonce, ciphertext) = buffer.split_at(NONCE_SIZE);

Figure 13: xmtp_mls/src/groups/message_history.rs

The above code does not ensure that the read file is at least NONCE_SIZE bytes long and may therefore panic when calling split_at(NONCE_SIZE). Given that this file is read from the file system, an attacker may therefore be able to crash the application via filesystem manipulation.

In general, panics should be avoided in situations that do not represent a truly unrecoverable state. When leveraged, panics should include useful information to the caller or user to enable debugging or troubleshooting of the underlying problem. In the above scenario, a check to ensure that the input is at least 12 (nonce) +16 (tag) = 28 bytes could be performed prior to parsing the nonce, with a suitable Err returned instead of a panic.

The above advice is consistent with the Secure Rust Guidelines:

Explicit error handling (Result) should always be preferred instead of calling panic. The cause of the error should be available, and generic errors should be avoided.

Recommendation

Consider either adding a length check and returning a suitable Err or adding an informative message to the panic to aid in debugging.

Location

xmtp_mls/src/groups/message_history.rs

xmtp / libxmtp