Skip to content

Commit f494dcd

Browse files
committed
Enforce size limit in payjoin-cli
Because these are unbounded allocations a malicious receiver could cause memory exhaustion for the sender. This approach uses a bytes_stream to avoid loading the entire buffer into memory before checking the size limit.
1 parent 7c9e7ee commit f494dcd

2 files changed

Lines changed: 26 additions & 8 deletions

File tree

payjoin-cli/Cargo.toml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,19 +32,21 @@ bitcoincore-rpc = "0.19.0"
3232
clap = { version = "~4.0.32", features = ["derive"] }
3333
config = "0.13.3"
3434
env_logger = "0.9.0"
35+
futures-util = "0.3"
3536
http-body-util = { version = "0.1", optional = true }
3637
hyper = { version = "1", features = ["http1", "server"], optional = true }
3738
hyper-rustls = { version = "0.26", optional = true }
3839
hyper-util = { version = "0.1", optional = true }
3940
log = "0.4.7"
4041
payjoin = { version = "0.23.0", default-features = false }
4142
rcgen = { version = "0.11.1", optional = true }
42-
reqwest = { version = "0.12", default-features = false }
43+
reqwest = { version = "0.12", default-features = false, features = ["stream"] }
4344
rustls = { version = "0.22.4", optional = true }
4445
serde = { version = "1.0.160", features = ["derive"] }
4546
sled = "0.34"
4647
tokio = { version = "1.38.1", features = ["full"] }
4748
tokio-rustls = { version = "0.25", features = ["ring"], default-features = false, optional = true }
49+
tokio-util = { version = "0.7", features = ["io", "io-util"] }
4850
url = { version = "2.3.1", features = ["serde"] }
4951

5052
[dev-dependencies]

payjoin-cli/src/app/v1.rs

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use std::sync::Arc;
55

66
use anyhow::{anyhow, Context, Result};
77
use bitcoincore_rpc::bitcoin::Amount;
8+
use futures_util::TryStreamExt;
89
use http_body_util::combinators::BoxBody;
910
use http_body_util::{BodyExt, Full};
1011
use hyper::body::{Buf, Bytes, Incoming};
@@ -17,9 +18,10 @@ use payjoin::bitcoin::FeeRate;
1718
use payjoin::receive::v1::{PayjoinProposal, UncheckedProposal};
1819
use payjoin::receive::ReplyableError::{self, Implementation, V1};
1920
use payjoin::send::v1::SenderBuilder;
20-
use payjoin::{ImplementationError, Uri, UriExt};
21+
use payjoin::{ImplementationError, Uri, UriExt, MAX_CONTENT_LENGTH};
2122
use tokio::net::TcpListener;
2223
use tokio::sync::watch;
24+
use tokio_util::io::{StreamReader, SyncIoBridge};
2325

2426
use super::config::Config;
2527
use super::wallet::BitcoindWallet;
@@ -88,12 +90,26 @@ impl AppTrait for App {
8890
"Sent fallback transaction hex: {:#}",
8991
payjoin::bitcoin::consensus::encode::serialize_hex(&fallback_tx)
9092
);
91-
let psbt = ctx.process_response(&mut response.bytes().await?.to_vec().as_slice()).map_err(
92-
|e| {
93-
log::debug!("Error processing response: {e:?}");
94-
anyhow!("Failed to process response {e}")
95-
},
96-
)?;
93+
94+
if let Some(content_length) = response.content_length() {
95+
if content_length > MAX_CONTENT_LENGTH as u64 {
96+
return Err(anyhow!(
97+
"Response content length exceeded the limit of {MAX_CONTENT_LENGTH} bytes"
98+
));
99+
}
100+
}
101+
102+
// Pass the response body to process_response without loading it all into memory.
103+
// This prevents a maliciously crafted response from causing an unbounded allocation.
104+
let stream =
105+
response.bytes_stream().map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e));
106+
let async_reader = StreamReader::new(stream);
107+
let mut sync_reader = SyncIoBridge::new(async_reader);
108+
109+
let psbt = ctx.process_response(&mut sync_reader).map_err(|e| {
110+
log::debug!("Error processing response: {e:?}");
111+
anyhow!("Failed to process response {}", e)
112+
})?;
97113

98114
self.process_pj_response(psbt)?;
99115
Ok(())

0 commit comments

Comments
 (0)