Skip to content

Commit 17d1ebe

Browse files
shinghimspacebear21
authored andcommitted
Limit response sizes for v1
BIP 78 doesn't enforce a size on the response, only that the content length must be included. However, if there is not a limit, then some of the send logic could make unbounded allocations.
1 parent c4bc5ce commit 17d1ebe

File tree

4 files changed

+79
-8
lines changed

4 files changed

+79
-8
lines changed

payjoin/src/lib.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,3 +76,7 @@ pub(crate) mod error_codes;
7676
pub use crate::core::error::ImplementationError;
7777
#[cfg(feature = "_core")]
7878
pub(crate) use crate::core::version::Version;
79+
80+
/// 4M block size limit with base64 encoding overhead => maximum reasonable size of content-length
81+
/// 4_000_000 * 4 / 3 fits in u32
82+
pub const MAX_CONTENT_LENGTH: usize = 4_000_000 * 4 / 3;

payjoin/src/receive/v1/exclusive/mod.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,8 @@ pub use error::RequestError;
44

55
use super::*;
66
use crate::into_url::IntoUrl;
7-
use crate::Version;
7+
use crate::{Version, MAX_CONTENT_LENGTH};
88

9-
/// 4_000_000 * 4 / 3 fits in u32
10-
const MAX_CONTENT_LENGTH: usize = 4_000_000 * 4 / 3;
119
const SUPPORTED_VERSIONS: &[Version] = &[Version::One];
1210

1311
pub trait Headers {

payjoin/src/send/error.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use bitcoin::transaction::Version;
66
use bitcoin::Sequence;
77

88
use crate::error_codes::ErrorCode;
9+
use crate::MAX_CONTENT_LENGTH;
910

1011
/// Error building a Sender from a SenderBuilder.
1112
///
@@ -95,6 +96,7 @@ pub struct ValidationError(InternalValidationError);
9596
pub(crate) enum InternalValidationError {
9697
Parse,
9798
Io(std::io::Error),
99+
ContentTooLarge,
98100
Proposal(InternalProposalError),
99101
#[cfg(feature = "v2")]
100102
V2Encapsulation(crate::send::v2::EncapsulationError),
@@ -119,6 +121,7 @@ impl fmt::Display for ValidationError {
119121
match &self.0 {
120122
Parse => write!(f, "couldn't decode as PSBT or JSON",),
121123
Io(e) => write!(f, "couldn't read PSBT: {e}"),
124+
ContentTooLarge => write!(f, "content is larger than {MAX_CONTENT_LENGTH} bytes"),
122125
Proposal(e) => write!(f, "proposal PSBT error: {e}"),
123126
#[cfg(feature = "v2")]
124127
V2Encapsulation(e) => write!(f, "v2 encapsulation error: {e}"),
@@ -133,6 +136,7 @@ impl std::error::Error for ValidationError {
133136
match &self.0 {
134137
Parse => None,
135138
Io(error) => Some(error),
139+
ContentTooLarge => None,
136140
Proposal(e) => Some(e),
137141
#[cfg(feature = "v2")]
138142
V2Encapsulation(e) => Some(e),

payjoin/src/send/v1.rs

Lines changed: 70 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
//! [`bitmask-core`](https://github.com/diba-io/bitmask-core) BDK integration. Bring your own
2222
//! wallet and http client.
2323
24+
use std::io::{BufRead, BufReader};
25+
2426
use bitcoin::psbt::Psbt;
2527
use bitcoin::{FeeRate, ScriptBuf, Weight};
2628
use error::{BuildSenderError, InternalBuildSenderError};
@@ -30,7 +32,7 @@ use super::*;
3032
pub use crate::output_substitution::OutputSubstitution;
3133
use crate::psbt::PsbtExt;
3234
use crate::request::Request;
33-
pub use crate::PjUri;
35+
use crate::{PjUri, MAX_CONTENT_LENGTH};
3436

3537
/// A builder to construct the properties of a `Sender`.
3638
#[derive(Clone)]
@@ -277,9 +279,15 @@ impl V1Context {
277279
self,
278280
response: &mut impl std::io::Read,
279281
) -> Result<Psbt, ResponseError> {
280-
let mut res_str = String::new();
281-
response.read_to_string(&mut res_str).map_err(InternalValidationError::Io)?;
282-
let proposal = Psbt::from_str(&res_str).map_err(|_| ResponseError::parse(&res_str))?;
282+
let mut buf_reader = BufReader::with_capacity(MAX_CONTENT_LENGTH + 1, response);
283+
let buffer = buf_reader.fill_buf().map_err(InternalValidationError::Io)?;
284+
285+
if buffer.len() > MAX_CONTENT_LENGTH {
286+
return Err(ResponseError::from(InternalValidationError::ContentTooLarge));
287+
}
288+
289+
let res_str = std::str::from_utf8(buffer).map_err(|_| InternalValidationError::Parse)?;
290+
let proposal = Psbt::from_str(res_str).map_err(|_| ResponseError::parse(res_str))?;
283291
self.psbt_context.process_proposal(proposal).map_err(Into::into)
284292
}
285293
}
@@ -289,7 +297,7 @@ mod test {
289297
use bitcoin::FeeRate;
290298
use payjoin_test_utils::{BoxError, PARSED_ORIGINAL_PSBT};
291299

292-
use super::SenderBuilder;
300+
use super::*;
293301
use crate::error_codes::ErrorCode;
294302
use crate::send::error::{ResponseError, WellKnownError};
295303
use crate::send::test::create_psbt_context;
@@ -298,6 +306,12 @@ mod test {
298306
const PJ_URI: &str =
299307
"bitcoin:2N47mmrWXsNBvQR6k78hWJoTji57zXwNcU7?amount=0.02&pjos=0&pj=HTTPS://EXAMPLE.COM/";
300308

309+
/// From the BIP-174 test vector
310+
const INVALID_PSBT: &str = "AgAAAAEmgXE3Ht/yhek3re6ks3t4AAwFZsuzrWRkFxPKQhcb9gAAAABqRzBEAiBwsiRRI+a/R01gxbUMBD1MaRpdJDXwmjSnZiqdwlF5CgIgATKcqdrPKAvfMHQOwDkEIkIsgctFg5RXrrdvwS7dlbMBIQJlfRGNM1e44PTCzUbbezn22cONmnCry5st5dyNv+TOMf7///8C09/1BQAAAAAZdqkU0MWZA8W6woaHYOkP1SGkZlqnZSCIrADh9QUAAAAAF6kUNUXm4zuDLEcFDyTT7rk8nAOUi8eHsy4TAA&#61;&#61;";
311+
312+
/// From the BIP-78 test vector
313+
const PJ_PROPOSAL_PSBT: &str = "cHNidP8BAJwCAAAAAo8nutGgJdyYGXWiBEb45Hoe9lWGbkxh/6bNiOJdCDuDAAAAAAD+////jye60aAl3JgZdaIERvjkeh72VYZuTGH/ps2I4l0IO4MBAAAAAP7///8CJpW4BQAAAAAXqRQd6EnwadJ0FQ46/q6NcutaawlEMIcACT0AAAAAABepFHdAltvPSGdDwi9DR+m0af6+i2d6h9MAAAAAAQEgqBvXBQAAAAAXqRTeTh6QYcpZE1sDWtXm1HmQRUNU0IcAAQEggIQeAAAAAAAXqRTI8sv5ymFHLIjkZNRrNXSEXZHY1YcBBxcWABRfgGZV5ZJMkgTC1RvlOU9L+e2iEAEIawJHMEQCIGe7e0DfJaVPRYEKWxddL2Pr0G37BoKz0lyNa02O2/tWAiB7ZVgBoF4s8MHocYWWmo4Q1cyV2wl7MX0azlqa8NBENAEhAmXWPPW0G3yE3HajBOb7gO7iKzHSmZ0o0w0iONowcV+tAAAA";
314+
301315
fn create_v1_context() -> super::V1Context {
302316
let psbt_context = create_psbt_context().expect("failed to create context");
303317
super::V1Context { psbt_context }
@@ -345,4 +359,55 @@ mod test {
345359
_ => panic!("Expected unrecognized JSON error"),
346360
}
347361
}
362+
363+
#[test]
364+
fn process_response_valid() {
365+
let mut cursor = std::io::Cursor::new(PJ_PROPOSAL_PSBT.as_bytes());
366+
367+
let ctx = create_v1_context();
368+
let response = ctx.process_response(&mut cursor);
369+
assert!(response.is_ok())
370+
}
371+
372+
#[test]
373+
fn process_response_invalid_psbt() {
374+
let mut cursor = std::io::Cursor::new(INVALID_PSBT.as_bytes());
375+
376+
let ctx = create_v1_context();
377+
let response = ctx.process_response(&mut cursor);
378+
match response {
379+
Ok(_) => panic!("Invalid PSBT should have caused an error"),
380+
Err(error) => match error {
381+
ResponseError::Validation(e) => {
382+
assert_eq!(
383+
e.to_string(),
384+
ValidationError::from(InternalValidationError::Parse).to_string()
385+
);
386+
}
387+
_ => panic!("Unexpected error type"),
388+
},
389+
}
390+
}
391+
392+
#[test]
393+
fn process_response_invalid_utf8() {
394+
// In UTF-8, 0xF0 represents the start of a 4-byte sequence, so 0xF0 by itself is invalid
395+
let invalid_utf8 = [0xF0];
396+
let mut cursor = std::io::Cursor::new(invalid_utf8);
397+
398+
let ctx = create_v1_context();
399+
let response = ctx.process_response(&mut cursor);
400+
match response {
401+
Ok(_) => panic!("Invalid UTF-8 should have caused an error"),
402+
Err(error) => match error {
403+
ResponseError::Validation(e) => {
404+
assert_eq!(
405+
e.to_string(),
406+
ValidationError::from(InternalValidationError::Parse).to_string()
407+
);
408+
}
409+
_ => panic!("Unexpected error type"),
410+
},
411+
}
412+
}
348413
}

0 commit comments

Comments
 (0)