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.

Author: shinghim

payjoin-cli/src/app/v1.rs

@@ -18,7 +18,7 @@ use payjoin::receive::v1::{PayjoinProposal, UncheckedProposal};
 use payjoin::receive::ImplementationError;
 use payjoin::receive::ReplyableError::{self, Implementation, V1};
 use payjoin::send::v1::SenderBuilder;
-use payjoin::{Uri, UriExt};
+use payjoin::{Uri, UriExt, MAX_CONTENT_LENGTH};
 use tokio::net::TcpListener;
 use tokio::sync::watch;
 
@@ -89,12 +89,15 @@ impl AppTrait for App {
             "Sent fallback transaction hex: {:#}",
             payjoin::bitcoin::consensus::encode::serialize_hex(&fallback_tx)
         );
-        let psbt = ctx.process_response(&mut response.bytes().await?.to_vec().as_slice()).map_err(
-            |e| {
-                log::debug!("Error processing response: {:?}", e);
-                anyhow!("Failed to process response {}", e)
-            },
-        )?;
+        let response_bytes = response.bytes().await?;
+        if response_bytes.len() > MAX_CONTENT_LENGTH {
+            return Err(anyhow!("Response bytes exceeded the limit of {MAX_CONTENT_LENGTH} bytes"));
+        }
+
+        let psbt = ctx.process_response(&mut response_bytes.to_vec().as_slice()).map_err(|e| {
+            log::debug!("Error processing response: {:?}", e);
+            anyhow!("Failed to process response {}", e)
+        })?;
 
         self.process_pj_response(psbt)?;
         Ok(())

payjoin/src/lib.rs

@@ -67,3 +67,7 @@ pub use uri::{PjParseError, PjUri, Uri, UriExt};
 pub use url::{ParseError, Url};
 #[cfg(feature = "_core")]
 pub(crate) mod error_codes;
+
+/// 4M block size limit with base64 encoding overhead => maximum reasonable size of content-length
+/// 4_000_000 * 4 / 3 fits in u32
+pub const MAX_CONTENT_LENGTH: usize = 4_000_000 * 4 / 3;

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

@@ -4,9 +4,8 @@ pub use error::RequestError;
 
 use super::*;
 use crate::into_url::IntoUrl;
+use crate::MAX_CONTENT_LENGTH;
 
-/// 4_000_000 * 4 / 3 fits in u32
-const MAX_CONTENT_LENGTH: usize = 4_000_000 * 4 / 3;
 const SUPPORTED_VERSIONS: &[usize] = &[1];
 
 pub trait Headers {

payjoin/src/send/error.rs

@@ -6,6 +6,7 @@ use bitcoin::transaction::Version;
 use bitcoin::Sequence;
 
 use crate::error_codes::ErrorCode;
+use crate::MAX_CONTENT_LENGTH;
 
 /// Error building a Sender from a SenderBuilder.
 ///
@@ -95,6 +96,7 @@ pub struct ValidationError(InternalValidationError);
 pub(crate) enum InternalValidationError {
     Parse,
     Io(std::io::Error),
+    ContentTooLarge,
     Proposal(InternalProposalError),
     #[cfg(feature = "v2")]
     V2Encapsulation(crate::send::v2::EncapsulationError),
@@ -119,6 +121,7 @@ impl fmt::Display for ValidationError {
         match &self.0 {
             Parse => write!(f, "couldn't decode as PSBT or JSON",),
             Io(e) => write!(f, "couldn't read PSBT: {}", e),
+            ContentTooLarge => write!(f, "content is larger than {} bytes", MAX_CONTENT_LENGTH),
             Proposal(e) => write!(f, "proposal PSBT error: {}", e),
             #[cfg(feature = "v2")]
             V2Encapsulation(e) => write!(f, "v2 encapsulation error: {}", e),
@@ -133,6 +136,7 @@ impl std::error::Error for ValidationError {
         match &self.0 {
             Parse => None,
             Io(error) => Some(error),
+            ContentTooLarge => None,
             Proposal(e) => Some(e),
             #[cfg(feature = "v2")]
             V2Encapsulation(e) => Some(e),

payjoin/src/send/v1.rs

@@ -21,6 +21,8 @@
 //! [`bitmask-core`](https://github.com/diba-io/bitmask-core) BDK integration. Bring your own
 //! wallet and http client.
 
+use std::io::{BufRead, BufReader};
+
 use bitcoin::psbt::Psbt;
 use bitcoin::{FeeRate, ScriptBuf, Weight};
 use error::{BuildSenderError, InternalBuildSenderError};
@@ -30,7 +32,7 @@ use super::*;
 pub use crate::output_substitution::OutputSubstitution;
 use crate::psbt::PsbtExt;
 use crate::request::Request;
-pub use crate::PjUri;
+use crate::{PjUri, MAX_CONTENT_LENGTH};
 
 #[derive(Clone)]
 pub struct SenderBuilder<'a> {
@@ -273,9 +275,15 @@ impl V1Context {
         self,
         response: &mut impl std::io::Read,
     ) -> Result<Psbt, ResponseError> {
-        let mut res_str = String::new();
-        response.read_to_string(&mut res_str).map_err(InternalValidationError::Io)?;
-        let proposal = Psbt::from_str(&res_str).map_err(|_| ResponseError::parse(&res_str))?;
+        let mut buf_reader = BufReader::with_capacity(MAX_CONTENT_LENGTH + 1, response);
+        let buffer = buf_reader.fill_buf().map_err(InternalValidationError::Io)?;
+
+        if buffer.len() > MAX_CONTENT_LENGTH {
+            return Err(ResponseError::from(InternalValidationError::ContentTooLarge));
+        }
+
+        let res_str = std::str::from_utf8(buffer).map_err(|_| InternalValidationError::Parse)?;
+        let proposal = Psbt::from_str(res_str).map_err(|_| ResponseError::parse(res_str))?;
         self.psbt_context.process_proposal(proposal).map_err(Into::into)
     }
 }
@@ -285,7 +293,7 @@ mod test {
     use bitcoin::FeeRate;
     use payjoin_test_utils::{BoxError, PARSED_ORIGINAL_PSBT};
 
-    use super::SenderBuilder;
+    use super::*;
     use crate::error_codes::ErrorCode;
     use crate::send::error::{ResponseError, WellKnownError};
     use crate::send::test::create_psbt_context;
@@ -294,6 +302,12 @@ mod test {
     const PJ_URI: &str =
         "bitcoin:2N47mmrWXsNBvQR6k78hWJoTji57zXwNcU7?amount=0.02&pjos=0&pj=HTTPS://EXAMPLE.COM/";
 
+    /// From the BIP-174 test vector
+    const INVALID_PSBT: &str = "AgAAAAEmgXE3Ht/yhek3re6ks3t4AAwFZsuzrWRkFxPKQhcb9gAAAABqRzBEAiBwsiRRI+a/R01gxbUMBD1MaRpdJDXwmjSnZiqdwlF5CgIgATKcqdrPKAvfMHQOwDkEIkIsgctFg5RXrrdvwS7dlbMBIQJlfRGNM1e44PTCzUbbezn22cONmnCry5st5dyNv+TOMf7///8C09/1BQAAAAAZdqkU0MWZA8W6woaHYOkP1SGkZlqnZSCIrADh9QUAAAAAF6kUNUXm4zuDLEcFDyTT7rk8nAOUi8eHsy4TAA&#61;&#61;";
+
+    /// From the BIP-78 test vector
+    const PJ_PROPOSAL_PSBT: &str = "cHNidP8BAJwCAAAAAo8nutGgJdyYGXWiBEb45Hoe9lWGbkxh/6bNiOJdCDuDAAAAAAD+////jye60aAl3JgZdaIERvjkeh72VYZuTGH/ps2I4l0IO4MBAAAAAP7///8CJpW4BQAAAAAXqRQd6EnwadJ0FQ46/q6NcutaawlEMIcACT0AAAAAABepFHdAltvPSGdDwi9DR+m0af6+i2d6h9MAAAAAAQEgqBvXBQAAAAAXqRTeTh6QYcpZE1sDWtXm1HmQRUNU0IcAAQEggIQeAAAAAAAXqRTI8sv5ymFHLIjkZNRrNXSEXZHY1YcBBxcWABRfgGZV5ZJMkgTC1RvlOU9L+e2iEAEIawJHMEQCIGe7e0DfJaVPRYEKWxddL2Pr0G37BoKz0lyNa02O2/tWAiB7ZVgBoF4s8MHocYWWmo4Q1cyV2wl7MX0azlqa8NBENAEhAmXWPPW0G3yE3HajBOb7gO7iKzHSmZ0o0w0iONowcV+tAAAA";
+
     fn create_v1_context() -> super::V1Context {
         let psbt_context = create_psbt_context().expect("failed to create context");
         super::V1Context { psbt_context }
@@ -341,4 +355,55 @@ mod test {
             _ => panic!("Expected unrecognized JSON error"),
         }
     }
+
+    #[test]
+    fn process_response_valid() {
+        let mut cursor = std::io::Cursor::new(PJ_PROPOSAL_PSBT.as_bytes());
+
+        let ctx = create_v1_context();
+        let response = ctx.process_response(&mut cursor);
+        assert!(response.is_ok())
+    }
+
+    #[test]
+    fn process_response_invalid_psbt() {
+        let mut cursor = std::io::Cursor::new(INVALID_PSBT.as_bytes());
+
+        let ctx = create_v1_context();
+        let response = ctx.process_response(&mut cursor);
+        match response {
+            Ok(_) => panic!("Invalid PSBT should have caused an error"),
+            Err(error) => match error {
+                ResponseError::Validation(e) => {
+                    assert_eq!(
+                        e.to_string(),
+                        ValidationError::from(InternalValidationError::Parse).to_string()
+                    );
+                }
+                _ => panic!("Unexpected error type"),
+            },
+        }
+    }
+
+    #[test]
+    fn process_response_invalid_utf8() {
+        // In UTF-8, 0xF0 represents the start of a 4-byte sequence, so 0xF0 by itself is invalid
+        let invalid_utf8 = [0xF0];
+        let mut cursor = std::io::Cursor::new(invalid_utf8);
+
+        let ctx = create_v1_context();
+        let response = ctx.process_response(&mut cursor);
+        match response {
+            Ok(_) => panic!("Invalid UTF-8 should have caused an error"),
+            Err(error) => match error {
+                ResponseError::Validation(e) => {
+                    assert_eq!(
+                        e.to_string(),
+                        ValidationError::from(InternalValidationError::Parse).to_string()
+                    );
+                }
+                _ => panic!("Unexpected error type"),
+            },
+        }
+    }
 }