From b2765cc324c85546e6bbae3ae6f97b7fad29db3d Mon Sep 17 00:00:00 2001 From: bwty Date: Thu, 15 Apr 2021 14:33:47 +0700 Subject: [PATCH 1/5] fix(packet): add regex check for numerical value before handing to chrono Signed-off-by: bwty test: datetime ensure error with invalid characters Signed-off-by: bwty doc: Prepare package datetime spec Signed-off-by: bwty Signed-off-by: bwty --- crates/interledger-packet/src/packet.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/crates/interledger-packet/src/packet.rs b/crates/interledger-packet/src/packet.rs index 4d86356a1..4e8dbe334 100644 --- a/crates/interledger-packet/src/packet.rs +++ b/crates/interledger-packet/src/packet.rs @@ -129,12 +129,28 @@ impl TryFrom for Prepare { type Error = ParseError; fn try_from(buffer: BytesMut) -> Result { + use once_cell::sync::OnceCell; + use regex::bytes::Regex; + let (content_offset, mut content) = deserialize_envelope(PacketType::Prepare, &buffer)?; let content_len = content.len(); let amount = content.read_u64::()?; + // Fixed Length DateTime format - RFC 0027 + // https://github.com/interledger/rfcs/blob/2dfdcf47ac52489a4ad473a5d869cd9f0217db67/0027-interledger-protocol-4/0027-interledger-protocol-4.md#ilp-prepare let mut expires_at = [0x00; 17]; content.read_exact(&mut expires_at)?; + + static RE: OnceCell = OnceCell::new(); + // Not using digit here as unicode feature needs to be enabled + let re = RE.get_or_init(|| Regex::new(r"[0-9]{17}").unwrap()); + + if !re.is_match(&expires_at) { + return Err(ParseError::InvalidPacket( + "DateTime must be numerical".into(), + )); + } + let expires_at = str::from_utf8(&expires_at[..])?; let expires_at: DateTime = Utc.datetime_from_str(&expires_at, INTERLEDGER_TIMESTAMP_FORMAT)?; @@ -741,6 +757,14 @@ mod test_prepare { assert!(Prepare::try_from(prep).is_err()); } + #[test] + fn test_invalid_datetime() { + let mut prep = BytesMut::from(PREPARE_BYTES); + // content offset = 4 bytes, amount is 8 bytes, datetime start at prep[12] + prep[12] = 42; // convert a byte from the datetime to a junk character + assert!(Prepare::try_from(prep).is_err()); + } + #[test] fn test_try_from() { assert_eq!( From e027d79283926c573fe39c6784018ff771bd4246 Mon Sep 17 00:00:00 2001 From: bwty Date: Thu, 15 Apr 2021 19:12:44 +0700 Subject: [PATCH 2/5] fix: test roundtrip for timestamp conversion Signed-off-by: bwty --- crates/interledger-packet/src/packet.rs | 29 ++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/crates/interledger-packet/src/packet.rs b/crates/interledger-packet/src/packet.rs index 4e8dbe334..783db0dd3 100644 --- a/crates/interledger-packet/src/packet.rs +++ b/crates/interledger-packet/src/packet.rs @@ -758,11 +758,30 @@ mod test_prepare { } #[test] - fn test_invalid_datetime() { - let mut prep = BytesMut::from(PREPARE_BYTES); - // content offset = 4 bytes, amount is 8 bytes, datetime start at prep[12] - prep[12] = 42; // convert a byte from the datetime to a junk character - assert!(Prepare::try_from(prep).is_err()); + fn test_invalid_ts() { + use std::convert::TryInto; + for i in 12..(12 + 17) { + let mut prep = BytesMut::from(PREPARE_BYTES); + prep[i] = 9; // convert a byte from the address to a junk character + let x = match Prepare::try_from(prep) { + Ok(x) => x, + Err(_) => { + continue; + } + }; + + let built = PrepareBuilder { + amount: x.amount(), + expires_at: x.expires_at(), + execution_condition: x.execution_condition().try_into().unwrap(), + destination: x.destination(), + data: x.data(), + } + .build(); + + assert_eq!(x, built); + assert_eq!(BytesMut::from(x), BytesMut::from(built)); + } } #[test] From ba121e13e59b5bfb67311f2b345a041b55b69743 Mon Sep 17 00:00:00 2001 From: bwty Date: Thu, 15 Apr 2021 21:08:49 +0700 Subject: [PATCH 3/5] fix: test with asserts Signed-off-by: bwty --- crates/interledger-packet/src/packet.rs | 25 +++---------------------- 1 file changed, 3 insertions(+), 22 deletions(-) diff --git a/crates/interledger-packet/src/packet.rs b/crates/interledger-packet/src/packet.rs index 783db0dd3..812e1cae2 100644 --- a/crates/interledger-packet/src/packet.rs +++ b/crates/interledger-packet/src/packet.rs @@ -146,9 +146,7 @@ impl TryFrom for Prepare { let re = RE.get_or_init(|| Regex::new(r"[0-9]{17}").unwrap()); if !re.is_match(&expires_at) { - return Err(ParseError::InvalidPacket( - "DateTime must be numerical".into(), - )); + return Err(ParseError::InvalidPacket("DateTime must be numeric".into())); } let expires_at = str::from_utf8(&expires_at[..])?; @@ -759,28 +757,11 @@ mod test_prepare { #[test] fn test_invalid_ts() { - use std::convert::TryInto; for i in 12..(12 + 17) { let mut prep = BytesMut::from(PREPARE_BYTES); prep[i] = 9; // convert a byte from the address to a junk character - let x = match Prepare::try_from(prep) { - Ok(x) => x, - Err(_) => { - continue; - } - }; - - let built = PrepareBuilder { - amount: x.amount(), - expires_at: x.expires_at(), - execution_condition: x.execution_condition().try_into().unwrap(), - destination: x.destination(), - data: x.data(), - } - .build(); - - assert_eq!(x, built); - assert_eq!(BytesMut::from(x), BytesMut::from(built)); + let err = Prepare::try_from(prep).unwrap_err(); + assert_eq!("Invalid Packet: DateTime must be numeric", &err.to_string()); } } From 3454bb47c2e4b3d26a2a79795c7939ddece3774c Mon Sep 17 00:00:00 2001 From: bwty Date: Fri, 16 Apr 2021 16:37:49 +0700 Subject: [PATCH 4/5] fix: remove regex for opt Signed-off-by: bwty --- crates/interledger-packet/src/packet.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/crates/interledger-packet/src/packet.rs b/crates/interledger-packet/src/packet.rs index 812e1cae2..dc4a489be 100644 --- a/crates/interledger-packet/src/packet.rs +++ b/crates/interledger-packet/src/packet.rs @@ -129,9 +129,6 @@ impl TryFrom for Prepare { type Error = ParseError; fn try_from(buffer: BytesMut) -> Result { - use once_cell::sync::OnceCell; - use regex::bytes::Regex; - let (content_offset, mut content) = deserialize_envelope(PacketType::Prepare, &buffer)?; let content_len = content.len(); let amount = content.read_u64::()?; @@ -141,11 +138,7 @@ impl TryFrom for Prepare { let mut expires_at = [0x00; 17]; content.read_exact(&mut expires_at)?; - static RE: OnceCell = OnceCell::new(); - // Not using digit here as unicode feature needs to be enabled - let re = RE.get_or_init(|| Regex::new(r"[0-9]{17}").unwrap()); - - if !re.is_match(&expires_at) { + if !expires_at.iter().all(|e| (b'0'..=b'9').contains(e)) { return Err(ParseError::InvalidPacket("DateTime must be numeric".into())); } From b0e9b99b93739c13cbff9e8436708be661ea0311 Mon Sep 17 00:00:00 2001 From: bwty Date: Fri, 16 Apr 2021 17:53:52 +0700 Subject: [PATCH 5/5] perf: optimise with constant time fn Signed-off-by: bwty --- crates/interledger-packet/src/packet.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/interledger-packet/src/packet.rs b/crates/interledger-packet/src/packet.rs index dc4a489be..1b5c14b53 100644 --- a/crates/interledger-packet/src/packet.rs +++ b/crates/interledger-packet/src/packet.rs @@ -138,7 +138,11 @@ impl TryFrom for Prepare { let mut expires_at = [0x00; 17]; content.read_exact(&mut expires_at)?; - if !expires_at.iter().all(|e| (b'0'..=b'9').contains(e)) { + if !expires_at + .iter() + .map(|e| (b'0'..=b'9').contains(e)) + .fold(true, |a, b| a & b) + { return Err(ParseError::InvalidPacket("DateTime must be numeric".into())); }