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/mod.rs

@@ -21,6 +21,9 @@ pub(crate) mod v2;
 #[cfg(feature = "_danger-local-https")]
 pub const LOCAL_CERT_FILE: &str = "localhost.der";
 
+/// 4_000_000 * 4 / 3 fits in u32
+pub(crate) const MAX_CONTENT_LENGTH: usize = 4_000_000 * 4 / 3;
+
 #[async_trait::async_trait]
 pub trait App: Send + Sync {
     fn new(config: Config) -> Result<Self>

payjoin-cli/src/app/v1.rs

@@ -24,7 +24,7 @@ use tokio::sync::watch;
 
 use super::config::Config;
 use super::wallet::BitcoindWallet;
-use super::App as AppTrait;
+use super::{App as AppTrait, MAX_CONTENT_LENGTH};
 use crate::app::{handle_interrupt, http_agent};
 use crate::db::Database;
 #[cfg(feature = "_danger-local-https")]
@@ -89,7 +89,12 @@ 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(
+        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)

payjoin/src/lib.rs

@@ -64,3 +64,7 @@ pub use uri::{PjParseError, PjUri, Uri, UriExt};
 pub use url::{ParseError, Url};
 #[cfg(feature = "_core")]
 pub(crate) mod error_codes;
+
+/// 4_000_000 * 4 / 3 fits in u32
+#[cfg(feature = "_core")]
+pub(crate) 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

@@ -7,6 +7,7 @@ use bitcoin::Sequence;
 use crate::error_codes::{
     NOT_ENOUGH_MONEY, ORIGINAL_PSBT_REJECTED, UNAVAILABLE, VERSION_UNSUPPORTED,
 };
+use crate::MAX_CONTENT_LENGTH;
 
 /// Error building a Sender from a SenderBuilder.
 ///
@@ -96,6 +97,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),
@@ -120,6 +122,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),
@@ -134,6 +137,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};
@@ -29,7 +31,7 @@ use url::Url;
 use super::*;
 use crate::psbt::PsbtExt;
 use crate::request::Request;
-use crate::PjUri;
+use crate::{PjUri, MAX_CONTENT_LENGTH};
 
 #[derive(Clone)]
 pub struct SenderBuilder<'a> {
@@ -272,9 +274,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)
     }
 }
@@ -283,6 +291,12 @@ impl V1Context {
 mod test {
     use crate::send::error::{ResponseError, WellKnownError};
 
+    /// From 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 {
         super::V1Context { psbt_context: crate::send::test::create_psbt_context() }
     }
@@ -311,4 +325,33 @@ 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).unwrap_err();
+        assert!(matches!(response, ResponseError::Validation(_)))
+    }
+
+    #[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).unwrap_err();
+        assert!(matches!(response, ResponseError::Validation(_)))
+    }
 }