Enforce content-length validation on sender and size limits on payjoin-cli

Author: GideonBature

payjoin-cli/src/app/v1.rs

@@ -29,6 +29,11 @@ use crate::db::Database;
 #[cfg(feature = "_danger-local-https")]
 pub const LOCAL_CERT_FILE: &str = "localhost.der";
 
+/// 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;
+
+#[derive(Clone)]
 struct Headers<'a>(&'a hyper::HeaderMap);
 impl payjoin::receive::v1::Headers for Headers<'_> {
     fn get_header(&self, key: &str) -> Option<&str> {
@@ -88,7 +93,19 @@ impl AppTrait for App {
             "Sent fallback transaction hex: {:#}",
             payjoin::bitcoin::consensus::encode::serialize_hex(&fallback_tx)
         );
-        let psbt = ctx.process_response(&response.bytes().await?).map_err(|e| {
+
+        let content_length = response
+            .headers()
+            .get("content-length")
+            .and_then(|val| val.to_str().ok())
+            .and_then(|s| s.parse::<usize>().ok());
+
+        if content_length.unwrap() > MAX_CONTENT_LENGTH {
+            log::debug!("Error in the size of content length");
+            return Err(anyhow!("Response body is too large: {:?} bytes", content_length));
+        }
+
+        let psbt = ctx.process_response(&response.bytes().await?, content_length).map_err(|e| {
             log::debug!("Error processing response: {e:?}");
             anyhow!("Failed to process response {e}")
         })?;
@@ -276,9 +293,24 @@ impl App {
     ) -> Result<Response<BoxBody<Bytes, hyper::Error>>, ReplyableError> {
         let (parts, body) = req.into_parts();
         let headers = Headers(&parts.headers);
+
+        let content_length = headers
+            .0
+            .get("content-length")
+            .and_then(|val| val.to_str().ok())
+            .and_then(|s| s.parse::<usize>().ok())
+            .unwrap();
+
+        if content_length > MAX_CONTENT_LENGTH {
+            log::error!("Error in the size of content length");
+            return Err(Implementation(
+                anyhow!("Content length too large: {content_length}").into(),
+            ));
+        };
+
         let query_string = parts.uri.query().unwrap_or("");
         let body = body.collect().await.map_err(|e| Implementation(e.into()))?.to_bytes();
-        let proposal = UncheckedProposal::from_request(&body, query_string, headers)?;
+        let proposal = UncheckedProposal::from_request(&body, query_string, headers.clone())?;
 
         let payjoin_proposal = self.process_v1_proposal(proposal)?;
         let psbt = payjoin_proposal.psbt();

payjoin-cli/src/app/v2/mod.rs

@@ -184,9 +184,16 @@ impl App {
                     Err(_) => {
                         let (req, v1_ctx) = context.extract_v1();
                         let response = post_request(req).await?;
-                        let psbt = Arc::new(
-                            v1_ctx.process_response(response.bytes().await?.to_vec().as_slice())?,
-                        );
+                        let content_length = response
+                            .headers()
+                            .get("content-length")
+                            .and_then(|val| val.to_str().ok())
+                            .and_then(|s| s.parse::<usize>().ok());
+
+                        let psbt = Arc::new(v1_ctx.process_response(
+                            response.bytes().await?.to_vec().as_slice(),
+                            content_length,
+                        )?);
                         self.process_pj_response((*psbt).clone())?;
                     }
                 }

payjoin/src/send/error.rs

@@ -6,7 +6,6 @@ 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,7 +94,10 @@ pub struct ValidationError(InternalValidationError);
 #[derive(Debug)]
 pub(crate) enum InternalValidationError {
     Parse,
-    ContentTooLarge,
+    ContentLengthMismatch {
+        expected: usize,
+        actual: usize,
+    },
     Proposal(InternalProposalError),
     #[cfg(feature = "v2")]
     V2Encapsulation(crate::send::v2::EncapsulationError),
@@ -119,7 +121,9 @@ impl fmt::Display for ValidationError {
 
         match &self.0 {
             Parse => write!(f, "couldn't decode as PSBT or JSON",),
-            ContentTooLarge => write!(f, "content is larger than {MAX_CONTENT_LENGTH} bytes"),
+            ContentLengthMismatch { expected, actual } => {
+                write!(f, "Content-Length mismatch. Expected {expected}, got {actual}")
+            }
             Proposal(e) => write!(f, "proposal PSBT error: {e}"),
             #[cfg(feature = "v2")]
             V2Encapsulation(e) => write!(f, "v2 encapsulation error: {e}"),
@@ -133,7 +137,7 @@ impl std::error::Error for ValidationError {
 
         match &self.0 {
             Parse => None,
-            ContentTooLarge => None,
+            ContentLengthMismatch { .. } => None,
             Proposal(e) => Some(e),
             #[cfg(feature = "v2")]
             V2Encapsulation(e) => Some(e),

payjoin/src/send/v1.rs

@@ -30,7 +30,7 @@ use super::*;
 pub use crate::output_substitution::OutputSubstitution;
 use crate::psbt::PsbtExt;
 use crate::request::Request;
-use crate::{PjUri, MAX_CONTENT_LENGTH};
+use crate::PjUri;
 
 /// A builder to construct the properties of a `Sender`.
 #[derive(Clone)]
@@ -278,9 +278,19 @@ impl V1Context {
     /// Call this method with response from receiver to continue BIP78 flow. If the response is
     /// valid you will get appropriate PSBT that you should sign and broadcast.
     #[inline]
-    pub fn process_response(self, response: &[u8]) -> Result<Psbt, ResponseError> {
-        if response.len() > MAX_CONTENT_LENGTH {
-            return Err(ResponseError::from(InternalValidationError::ContentTooLarge));
+    pub fn process_response(
+        self,
+        response: &[u8],
+        content_length: Option<usize>,
+    ) -> Result<Psbt, ResponseError> {
+        if let Some(expected_len) = content_length {
+            if response.len() != expected_len {
+                return Err(InternalValidationError::ContentLengthMismatch {
+                    expected: expected_len,
+                    actual: response.len(),
+                }
+                .into());
+            }
         }
 
         let res_str = std::str::from_utf8(response).map_err(|_| InternalValidationError::Parse)?;
@@ -426,7 +436,9 @@ mod test {
             "message": "This version of payjoin is not supported."
         })
         .to_string();
-        match ctx.process_response(known_json_error.as_bytes()) {
+
+        let content_length = Some(known_json_error.len());
+        match ctx.process_response(known_json_error.as_bytes(), content_length) {
             Err(ResponseError::WellKnown(WellKnownError {
                 code: ErrorCode::VersionUnsupported,
                 ..
@@ -440,7 +452,9 @@ mod test {
             "message": "This version of payjoin is not supported."
         })
         .to_string();
-        match ctx.process_response(invalid_json_error.as_bytes()) {
+
+        let content_length = Some(invalid_json_error.len());
+        match ctx.process_response(invalid_json_error.as_bytes(), content_length) {
             Err(ResponseError::Validation(_)) => (),
             _ => panic!("Expected unrecognized JSON error"),
         }
@@ -449,14 +463,20 @@ mod test {
     #[test]
     fn process_response_valid() {
         let ctx = create_v1_context();
-        let response = ctx.process_response(PAYJOIN_PROPOSAL.as_bytes());
+        let body = PAYJOIN_PROPOSAL.as_bytes();
+
+        let content_length = Some(body.len());
+        let response = ctx.process_response(PAYJOIN_PROPOSAL.as_bytes(), content_length);
         assert!(response.is_ok())
     }
 
     #[test]
     fn process_response_invalid_psbt() {
         let ctx = create_v1_context();
-        let response = ctx.process_response(INVALID_PSBT.as_bytes());
+        let body = INVALID_PSBT.as_bytes();
+
+        let content_length = Some(body.len());
+        let response = ctx.process_response(INVALID_PSBT.as_bytes(), content_length);
         match response {
             Ok(_) => panic!("Invalid PSBT should have caused an error"),
             Err(error) => match error {
@@ -477,7 +497,9 @@ mod test {
         let invalid_utf8 = &[0xF0];
 
         let ctx = create_v1_context();
-        let response = ctx.process_response(invalid_utf8);
+
+        let content_length = Some(invalid_utf8.len());
+        let response = ctx.process_response(invalid_utf8, content_length);
         match response {
             Ok(_) => panic!("Invalid UTF-8 should have caused an error"),
             Err(error) => match error {
@@ -494,18 +516,22 @@ mod test {
 
     #[test]
     fn process_response_invalid_buffer_len() {
-        let mut data = PAYJOIN_PROPOSAL.as_bytes().to_vec();
-        data.extend(std::iter::repeat(0).take(MAX_CONTENT_LENGTH + 1));
+        let data = PAYJOIN_PROPOSAL.as_bytes().to_vec();
+        let bad_content_length = data.len() + 10;
 
         let ctx = create_v1_context();
-        let response = ctx.process_response(&data);
+        let response = ctx.process_response(&data, Some(bad_content_length));
         match response {
             Ok(_) => panic!("Invalid buffer length should have caused an error"),
             Err(error) => match error {
                 ResponseError::Validation(e) => {
                     assert_eq!(
                         e.to_string(),
-                        ValidationError::from(InternalValidationError::ContentTooLarge).to_string()
+                        ValidationError::from(InternalValidationError::ContentLengthMismatch {
+                            expected: bad_content_length,
+                            actual: data.len()
+                        })
+                        .to_string()
                     );
                 }
                 _ => panic!("Unexpected error type"),

payjoin/tests/integration.rs

@@ -100,7 +100,10 @@ mod integration {
             // **********************
             // Inside the Sender:
             // Sender checks, signs, finalizes, extracts, and broadcasts
-            let checked_payjoin_proposal_psbt = ctx.process_response(response.as_bytes())?;
+            let response_body = response.as_bytes();
+            let content_length = Some(response_body.len());
+            let checked_payjoin_proposal_psbt =
+                ctx.process_response(response_body, content_length)?;
             let payjoin_tx = extract_pj_tx(&sender, checked_payjoin_proposal_psbt)?;
             sender.send_raw_transaction(&payjoin_tx)?;
 
@@ -585,7 +588,10 @@ mod integration {
             // **********************
             // Inside the Sender:
             // Sender checks, signs, finalizes, extracts, and broadcasts
-            let checked_payjoin_proposal_psbt = ctx.process_response(response.as_bytes())?;
+            let response_body = response.as_bytes();
+            let content_length = Some(response_body.len());
+            let checked_payjoin_proposal_psbt =
+                ctx.process_response(response_body, content_length)?;
             let payjoin_tx = extract_pj_tx(&sender, checked_payjoin_proposal_psbt)?;
             sender.send_raw_transaction(&payjoin_tx)?;
 
@@ -710,7 +716,7 @@ mod integration {
                 assert!(response.status().is_success(), "error response: {}", response.status());
 
                 let checked_payjoin_proposal_psbt =
-                    send_ctx.process_response(&response.bytes().await?)?;
+                    send_ctx.process_response(&response.bytes().await?, None)?;
                 let payjoin_tx = extract_pj_tx(&sender, checked_payjoin_proposal_psbt)?;
                 sender.send_raw_transaction(&payjoin_tx)?;
                 log::info!("sent");
@@ -1211,7 +1217,10 @@ mod integration {
             // **********************
             // Inside the Sender:
             // Sender checks, signs, finalizes, extracts, and broadcasts
-            let checked_payjoin_proposal_psbt = ctx.process_response(response.as_bytes())?;
+            let response_body = response.as_bytes();
+            let content_length = Some(response_body.len());
+            let checked_payjoin_proposal_psbt =
+                ctx.process_response(response_body, content_length)?;
             let payjoin_tx = extract_pj_tx(&sender, checked_payjoin_proposal_psbt)?;
             sender.send_raw_transaction(&payjoin_tx)?;
 
@@ -1297,7 +1306,10 @@ mod integration {
             // **********************
             // Inside the Sender:
             // Sender checks, signs, finalizes, extracts, and broadcasts
-            let checked_payjoin_proposal_psbt = ctx.process_response(response.as_bytes())?;
+            let response_body = response.as_bytes();
+            let content_length = Some(response_body.len());
+            let checked_payjoin_proposal_psbt =
+                ctx.process_response(response_body, content_length)?;
             let payjoin_tx = extract_pj_tx(&sender, checked_payjoin_proposal_psbt)?;
             sender.send_raw_transaction(&payjoin_tx)?;