xmtp / libxmtp

LibXMTP is a shared library encapsulating the core functionality of the XMTP messaging protocol, such as cryptography, networking, and language bindings.
MIT License
45 stars 19 forks source link

Remove insecure use of temporary directory #1186

Open neekolas opened 4 hours ago

neekolas commented 4 hours ago

Impact

A well-positioned attacker may be able to obtain a copy of a user’s message history, overwrite files, or inject arbitrary groups or messages into the user’s database.

Description

In libxmtp/xmtp_mls/src/groups/message_history.rs, message history bundles are read and written through subpaths of std::env::temp_dir(). However, the application and the running user are not guaranteed exclusive use of the temp_dir() folder. From the Rust docs:

The temporary directory may be shared among users, or between processes with different privileges; thus, the creation of any files or directories in the temporary directory must use a secure method to create a uniquely named file. Creating a file or directory with a fixed or predictable name may result in “insecure temporary file”^2 security vulnerabilities. Consider using a crate that securely creates temporary files or directories.

In fact, libxmtp does use predictable file names, as in the following excerpt:


async fn write_history_bundle(&self) -> Result<(PathBuf, HistoryKeyType), 
MessageHistoryError> {
    let groups = self.prepare_groups_to_sync().await?;
    let messages = self.prepare_messages_to_sync().await?;

    let temp_file = std::env::temp_dir().join("history.jsonl.tmp");
    write_to_file(temp_file.as_path(), groups)?;
    write_to_file(temp_file.as_path(), messages)?;

    let history_file = std::env::temp_dir().join("history.jsonl.enc");
    let enc_key = HistoryKeyType::new_chacha20_poly1305_key();
    encrypt_history_file(
        temp_file.as_path(),
        history_file.as_path(),
        enc_key.as_bytes(),
    )?;

    std::fs::remove_file(temp_file.as_path())?;

    Ok((history_file, enc_key))
}
codabrink commented 4 hours ago

This is fixed in https://github.com/xmtp/libxmtp/pull/1152 I refactored it to skip the fs altogether.