Limit response sizes for v1 (payjoin#586)

Add content length checks in v1 send and payjoin-cli.

Fixes payjoin#483

Author: spacebear21

payjoin-test-utils/src/lib.rs

@@ -267,7 +267,6 @@ pub const KEM: Kem = Kem::K256Sha256;
 pub const SYMMETRIC: &[SymmetricSuite] =
     &[ohttp::SymmetricSuite::new(Kdf::HkdfSha256, Aead::ChaCha20Poly1305)];
 
-// OriginalPSBT Test Vector from BIP 78
 // https://github.com/bitcoin/bips/blob/master/bip-0078.mediawiki#user-content-span_idtestvectorsspanTest_vectors
 // | InputScriptType | Original PSBT Fee rate | maxadditionalfeecontribution | additionalfeeoutputindex|
 // |-----------------|-----------------------|------------------------------|-------------------------|

payjoin/src/lib.rs

@@ -76,3 +76,7 @@ pub(crate) mod error_codes;
 pub use crate::core::error::ImplementationError;
 #[cfg(feature = "_core")]
 pub(crate) use crate::core::version::Version;
+
+/// 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,10 +4,8 @@ pub use error::RequestError;
 
 use super::*;
 use crate::into_url::IntoUrl;
-use crate::Version;
+use crate::{Version, MAX_CONTENT_LENGTH};
 
-/// 4_000_000 * 4 / 3 fits in u32
-const MAX_CONTENT_LENGTH: usize = 4_000_000 * 4 / 3;
 const SUPPORTED_VERSIONS: &[Version] = &[Version::One];
 
 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 {MAX_CONTENT_LENGTH} bytes"),
             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};
 
 /// A builder to construct the properties of a `Sender`.
 #[derive(Clone)]
@@ -277,19 +279,25 @@ 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)
     }
 }
 
 #[cfg(test)]
 mod test {
     use bitcoin::FeeRate;
-    use payjoin_test_utils::{BoxError, PARSED_ORIGINAL_PSBT};
+    use payjoin_test_utils::{BoxError, INVALID_PSBT, PARSED_ORIGINAL_PSBT, PAYJOIN_PROPOSAL};
 
-    use super::SenderBuilder;
+    use super::*;
     use crate::error_codes::ErrorCode;
     use crate::send::error::{ResponseError, WellKnownError};
     use crate::send::test::create_psbt_context;
@@ -345,4 +353,55 @@ mod test {
             _ => panic!("Expected unrecognized JSON error"),
         }
     }
+
+    #[test]
+    fn process_response_valid() {
+        let mut cursor = std::io::Cursor::new(PAYJOIN_PROPOSAL.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"),
+            },
+        }
+    }
 }