From b785b13a659512234e851273e0453081a1f0907d Mon Sep 17 00:00:00 2001 From: John Beisley Date: Sat, 6 Jul 2024 09:48:24 +0100 Subject: [PATCH] fix: restrict which characters can be used in fingerprint components --- src/fingerprint.rs | 35 ++++++++++++++++++++++++++++--- src/importers/nationwide_csv.rs | 37 +++++++++++++++++++++------------ src/importers/nationwide_pdf.rs | 25 +++++++++++----------- src/importers/paypal_csv.rs | 2 +- 4 files changed, 70 insertions(+), 29 deletions(-) diff --git a/src/fingerprint.rs b/src/fingerprint.rs index bed8d2c..9d0afce 100644 --- a/src/fingerprint.rs +++ b/src/fingerprint.rs @@ -1,6 +1,9 @@ +use anyhow::{bail, Result}; use byteorder::{BigEndian, ByteOrder}; use chrono::{Datelike, NaiveDate, NaiveTime, Timelike}; +use lazy_static::lazy_static; use ledger_parser::Amount; +use regex::Regex; use sha1::{Digest, Sha1}; use crate::tags; @@ -53,13 +56,39 @@ pub struct FingerprintBuilder { } impl FingerprintBuilder { - pub fn new(algorithm_name: &'static str, algorithm_version: i64, user_namespace: &str) -> Self { - Self { + pub fn new( + algorithm_name: &'static str, + algorithm_version: i64, + user_namespace: &str, + ) -> Result { + lazy_static! { + // This regex should only allow characters that don't conflict with + // the tag structure generated by [Fingerprint::tag]. + // Including '+' and '/' allow for standard base64 encoding. + static ref VALID_FP_PART_RX: Regex = Regex::new(r#"^[a-zA-Z0-9_+/]*$"#).unwrap(); + } + + if !VALID_FP_PART_RX.is_match(&algorithm_name) { + bail!( + "fingerprint algorithm name {:?} must match regex {:?}", + algorithm_name, + VALID_FP_PART_RX.as_str() + ); + } + if !VALID_FP_PART_RX.is_match(user_namespace) { + bail!( + "fingerprint user namespace {:?} must match regex {:?}", + user_namespace, + VALID_FP_PART_RX.as_str() + ); + } + + Ok(Self { acc: Accumulator::new(), algorithm_name, algorithm_version, user_namespace: user_namespace.to_string(), - } + }) } pub fn build(self) -> Fingerprint { diff --git a/src/importers/nationwide_csv.rs b/src/importers/nationwide_csv.rs index f4299c2..a9ad1d2 100644 --- a/src/importers/nationwide_csv.rs +++ b/src/importers/nationwide_csv.rs @@ -210,7 +210,7 @@ impl PostingFormer for RecordFive { _ => bail!("expected *either* paid in or paid out"), }; let halves = self_and_peer_account_amount(self_amount, ASSETS_UNKNOWN.to_string()); - let fp_v1 = self.fingerprint_v1(fp_namespace, date_counter); + let fp_v1 = self.fingerprint_v1(fp_namespace, date_counter)?; let mut self_comment = Comment::builder() .with_tag(tags::UNKNOWN_ACCOUNT) .with_value_tag(tags::ACCOUNT, account_name) @@ -281,7 +281,7 @@ impl PostingFormer for RecordSix { .with_value_tag(tags::BANK, BANK_NAME) .with_value_tag(TRANSACTION_TYPE_TAG, self.type_.clone()); let mut peer_comment = self_comment.clone(); - let fp_v1 = self.fingerprint_v1(fp_namespace, date_counter); + let fp_v1 = self.fingerprint_v1(fp_namespace, date_counter)?; self_comment = self_comment .with_tag(fp_v1.self_.tag()) .with_tag(tags::IMPORT_SELF.to_string()); @@ -289,7 +289,7 @@ impl PostingFormer for RecordSix { .with_tag(fp_v1.peer.tag()) .with_tag(tags::IMPORT_PEER.to_string()); if include_legacy_fingerprint { - let fp_legacy = self.fingerprint_legacy(fp_namespace, date_counter, &halves); + let fp_legacy = self.fingerprint_legacy(fp_namespace, date_counter, &halves)?; self_comment = self_comment.with_tag(fp_legacy.self_.legacy_tag()); peer_comment = peer_comment.with_tag(fp_legacy.peer.legacy_tag()); } @@ -318,7 +318,7 @@ mod de { use std::fmt; use std::str::FromStr; - use anyhow::{bail, Result}; + use anyhow::{bail, Context, Result}; use chrono::NaiveDate; use lazy_static::lazy_static; use ledger_parser::{Amount, Commodity, CommodityPosition}; @@ -345,16 +345,21 @@ mod de { } impl RecordFive { - pub fn fingerprint_v1(&self, fp_namespace: &str, date_counter: i32) -> FingerprintHalves { - self_and_peer_fingerprints( + pub fn fingerprint_v1( + &self, + fp_namespace: &str, + date_counter: i32, + ) -> Result { + Ok(self_and_peer_fingerprints( FingerprintBuilder::new("nwcsv5", 1, fp_namespace) + .with_context(|| "building v1 fingerprint")? .with(self.date.0) .with(date_counter) .with(self.transactions.as_str()) .with(self.location.as_str()) .with(self.paid_out.as_ref()) .with(self.paid_in.as_ref()), - ) + )) } } @@ -378,8 +383,9 @@ mod de { fp_namespace: &str, date_counter: i32, halves: &TransactionHalves, - ) -> FingerprintHalves { + ) -> Result { let fpb_legacy = FingerprintBuilder::new("", 0, fp_namespace) + .with_context(|| "building legacy fingerprint")? .with(self.type_.as_str()) .with(self.date.0) // Description should have been included in the legacy fingerprint, but a @@ -396,15 +402,20 @@ mod de { .with(halves.peer.account.as_str()) .with(&halves.peer.amount); - FingerprintHalves { + Ok(FingerprintHalves { self_: self_fp.build(), peer: peer_fp.build(), - } + }) } - pub fn fingerprint_v1(&self, fp_namespace: &str, date_counter: i32) -> FingerprintHalves { - self_and_peer_fingerprints( + pub fn fingerprint_v1( + &self, + fp_namespace: &str, + date_counter: i32, + ) -> Result { + Ok(self_and_peer_fingerprints( FingerprintBuilder::new("nwcsv6", 1, fp_namespace) + .with_context(|| "building v1 fingerprint")? .with(self.type_.as_str()) .with(self.date.0) .with(date_counter) @@ -412,7 +423,7 @@ mod de { .with(self.paid_out.as_ref()) .with(self.paid_in.as_ref()) .with(&self.balance), - ) + )) } } diff --git a/src/importers/nationwide_pdf.rs b/src/importers/nationwide_pdf.rs index 318a035..5784c76 100644 --- a/src/importers/nationwide_pdf.rs +++ b/src/importers/nationwide_pdf.rs @@ -65,7 +65,7 @@ impl TransactionImporter for NationwidePdf { } } - let transactions = acc.build(); + let transactions = acc.build()?; Ok(Import { user_fp_namespace, transactions, @@ -208,7 +208,7 @@ impl TransactionsAccumulator { } (Some(payment), None) => { // Start of new payment transaction. - self.flush_transaction(); + self.flush_transaction()?; if trn_line.implied_date != self.prev_date { self.date_counter = 0; } else { @@ -224,7 +224,7 @@ impl TransactionsAccumulator { } (None, Some(receipt)) => { // Start of new receipt transaction. - self.flush_transaction(); + self.flush_transaction()?; if trn_line.implied_date != self.prev_date { self.date_counter = 0; } else { @@ -276,15 +276,16 @@ impl TransactionsAccumulator { Ok(()) } - fn flush_transaction(&mut self) { + fn flush_transaction(&mut self) -> Result<()> { if let Some(pending) = self.cur_trn_opt.take() { - self.trns.push(pending.build(&self.fp_ns)); + self.trns.push(pending.build(&self.fp_ns)?); } + Ok(()) } - fn build(mut self) -> Vec { - self.flush_transaction(); - self.trns + fn build(mut self) -> Result> { + self.flush_transaction()?; + Ok(self.trns) } } @@ -332,8 +333,8 @@ impl TransactionBuilder { }) } - fn build(self, fp_ns: &str) -> Transaction { - let record_fpb = FingerprintBuilder::new("nwpdf", 1, fp_ns) + fn build(self, fp_ns: &str) -> Result { + let record_fpb = FingerprintBuilder::new("nwpdf", 1, fp_ns)? .with(self.date) .with(self.date_counter) .with(self.description.as_str()); @@ -357,7 +358,7 @@ impl TransactionBuilder { .with(halves.peer.account.as_str()) .with(&halves.peer.amount); - Transaction { + Ok(Transaction { date: self.date, effective_date: self.effective_date, status: None, @@ -391,7 +392,7 @@ impl TransactionBuilder { .into_opt_comment(), }, ], - } + }) } } diff --git a/src/importers/paypal_csv.rs b/src/importers/paypal_csv.rs index 531f224..09c52a9 100644 --- a/src/importers/paypal_csv.rs +++ b/src/importers/paypal_csv.rs @@ -188,7 +188,7 @@ impl Record { quantity: v.balance, commodity, }; - let partial_fp = FingerprintBuilder::new("ppcsv", 1, fp_ns) + let partial_fp = FingerprintBuilder::new("ppcsv", 1, fp_ns)? .with(v.date.0) .with(v.time.0) .with(v.time_zone.as_str())