Skip to content

Commit f48867e

Browse files
KenbakKenbak
committed
fix: harden donation mode — URL protocol validation, image position allowlist, config merge
- Add validate_url_protocol() and validate_image_position() to validation.rs - Apply protocol checks to website_url, cover_image_url, success_url in both create and update paths (closes XSS via javascript: href on PATCH) - Allowlist cover_image_position to prevent arbitrary CSS injection - Merge donation_config fields on update instead of replacing entire object - Rate-limit the public /info endpoint - Add tests for new validation helpers
1 parent 7c094e9 commit f48867e

4 files changed

Lines changed: 94 additions & 4 deletions

File tree

‎src/api/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ pub fn configure(cfg: &mut web::ServiceConfig) {
9191
.route("/payment-links/{id}", web::patch().to(payment_links::update))
9292
.route("/payment-links/{id}", web::delete().to(payment_links::delete))
9393
.route("/payment-links/{slug}/checkout", web::post().to(payment_links::resolve).wrap(Governor::new(&session_rate_limit)))
94-
.route("/payment-links/{slug}/info", web::get().to(payment_links::info))
94+
.route("/payment-links/{slug}/info", web::get().to(payment_links::info).wrap(Governor::new(&session_rate_limit)))
9595
// Donation links (merchant auth)
9696
.route("/donation-links", web::post().to(payment_links::create_donation))
9797
// Buyer checkout (public)

‎src/api/payment_links.rs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -604,18 +604,24 @@ fn validate_create_donation(req: &CreateDonationLinkRequest) -> Result<(), valid
604604
}
605605
if let Some(ref url) = req.cover_image_url {
606606
validation::validate_length("cover_image_url", url, 2000)?;
607+
validation::validate_url_protocol("cover_image_url", url, false)?;
608+
}
609+
if let Some(ref pos) = req.cover_image_position {
610+
validation::validate_image_position("cover_image_position", pos)?;
607611
}
608612
if let Some(ref email) = req.contact_email {
609613
validation::validate_length("contact_email", email, 200)?;
610614
}
611615
if let Some(ref url) = req.website_url {
612616
validation::validate_length("website_url", url, 2000)?;
617+
validation::validate_url_protocol("website_url", url, true)?;
613618
}
614619
if let Some(ref text) = req.social_share_text {
615620
validation::validate_length("social_share_text", text, 500)?;
616621
}
617622
if let Some(ref url) = req.success_url {
618623
validation::validate_length("success_url", url, 2000)?;
624+
validation::validate_url_protocol("success_url", url, true)?;
619625
}
620626
Ok(())
621627
}
@@ -626,6 +632,9 @@ fn validate_update(req: &UpdatePaymentLinkRequest) -> Result<(), validation::Val
626632
}
627633
if let Some(ref url) = req.success_url {
628634
validation::validate_length("success_url", url, 2000)?;
635+
if !url.is_empty() {
636+
validation::validate_url_protocol("success_url", url, true)?;
637+
}
629638
}
630639
if let Some(ref dc) = req.donation_config {
631640
if let Some(ref mission) = dc.mission {
@@ -639,12 +648,21 @@ fn validate_update(req: &UpdatePaymentLinkRequest) -> Result<(), validation::Val
639648
}
640649
if let Some(ref url) = dc.cover_image_url {
641650
validation::validate_length("cover_image_url", url, 2000)?;
651+
if !url.is_empty() {
652+
validation::validate_url_protocol("cover_image_url", url, false)?;
653+
}
654+
}
655+
if let Some(ref pos) = dc.cover_image_position {
656+
validation::validate_image_position("cover_image_position", pos)?;
642657
}
643658
if let Some(ref email) = dc.contact_email {
644659
validation::validate_length("contact_email", email, 200)?;
645660
}
646661
if let Some(ref url) = dc.website_url {
647662
validation::validate_length("website_url", url, 2000)?;
663+
if !url.is_empty() {
664+
validation::validate_url_protocol("website_url", url, true)?;
665+
}
648666
}
649667
if let Some(ref text) = dc.social_share_text {
650668
validation::validate_length("social_share_text", text, 500)?;

‎src/payment_links/mod.rs‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -289,9 +289,33 @@ pub async fn update_payment_link(
289289
.or(existing.metadata);
290290

291291
let donation_config_json = if existing.mode == "donation" {
292-
req.donation_config.as_ref()
293-
.map(|dc| serde_json::to_string(dc).unwrap_or_default())
294-
.or(existing.donation_config)
292+
match (&req.donation_config, &existing.donation_config) {
293+
(Some(updates), Some(existing_json)) => {
294+
let mut base: DonationConfig = serde_json::from_str(existing_json)
295+
.unwrap_or(DonationConfig {
296+
mission: None, thank_you: None, suggested_amounts: None,
297+
currency: "USD".to_string(), min_amount: None, max_amount: None,
298+
campaign_name: None, campaign_goal: None, cover_image_url: None,
299+
cover_image_position: None, contact_email: None, website_url: None,
300+
social_share_text: None,
301+
});
302+
if updates.mission.is_some() { base.mission = updates.mission.clone(); }
303+
if updates.thank_you.is_some() { base.thank_you = updates.thank_you.clone(); }
304+
if updates.suggested_amounts.is_some() { base.suggested_amounts = updates.suggested_amounts.clone(); }
305+
if updates.min_amount.is_some() { base.min_amount = updates.min_amount; }
306+
if updates.max_amount.is_some() { base.max_amount = updates.max_amount; }
307+
if updates.campaign_name.is_some() { base.campaign_name = updates.campaign_name.clone(); }
308+
if updates.campaign_goal.is_some() { base.campaign_goal = updates.campaign_goal; }
309+
if updates.cover_image_url.is_some() { base.cover_image_url = updates.cover_image_url.clone(); }
310+
if updates.cover_image_position.is_some() { base.cover_image_position = updates.cover_image_position.clone(); }
311+
if updates.contact_email.is_some() { base.contact_email = updates.contact_email.clone(); }
312+
if updates.website_url.is_some() { base.website_url = updates.website_url.clone(); }
313+
if updates.social_share_text.is_some() { base.social_share_text = updates.social_share_text.clone(); }
314+
Some(serde_json::to_string(&base).unwrap_or_default())
315+
}
316+
(Some(dc), None) => Some(serde_json::to_string(dc).unwrap_or_default()),
317+
(None, existing_dc) => existing_dc.clone(),
318+
}
295319
} else {
296320
existing.donation_config
297321
};

‎src/validation.rs‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,33 @@ pub fn validate_optional_length(
4747
Ok(())
4848
}
4949

50+
pub fn validate_url_protocol(field: &str, url: &str, allow_http: bool) -> Result<(), ValidationError> {
51+
if url.starts_with("https://") {
52+
return Ok(());
53+
}
54+
if allow_http && url.starts_with("http://") {
55+
return Ok(());
56+
}
57+
let msg = if allow_http {
58+
"must start with http:// or https://"
59+
} else {
60+
"must start with https://"
61+
};
62+
Err(ValidationError::invalid(field, msg))
63+
}
64+
65+
const ALLOWED_IMAGE_POSITIONS: &[&str] = &[
66+
"center top", "center center", "center bottom",
67+
];
68+
69+
pub fn validate_image_position(field: &str, value: &str) -> Result<(), ValidationError> {
70+
if ALLOWED_IMAGE_POSITIONS.contains(&value) {
71+
Ok(())
72+
} else {
73+
Err(ValidationError::invalid(field, "must be one of: center top, center center, center bottom"))
74+
}
75+
}
76+
5077
pub fn validate_email_format(field: &str, email: &str) -> Result<(), ValidationError> {
5178
validate_length(field, email, 254)?;
5279

@@ -286,6 +313,27 @@ mod tests {
286313
assert!(validate_zcash_address("addr", "t1000000000000000000000000000000000").is_err());
287314
}
288315

316+
#[test]
317+
fn test_validate_url_protocol() {
318+
assert!(validate_url_protocol("url", "https://example.com", false).is_ok());
319+
assert!(validate_url_protocol("url", "http://example.com", false).is_err());
320+
assert!(validate_url_protocol("url", "http://example.com", true).is_ok());
321+
assert!(validate_url_protocol("url", "javascript:alert(1)", false).is_err());
322+
assert!(validate_url_protocol("url", "javascript:alert(1)", true).is_err());
323+
assert!(validate_url_protocol("url", "data:text/html,<h1>hi</h1>", false).is_err());
324+
assert!(validate_url_protocol("url", "ftp://example.com", true).is_err());
325+
}
326+
327+
#[test]
328+
fn test_validate_image_position() {
329+
assert!(validate_image_position("pos", "center top").is_ok());
330+
assert!(validate_image_position("pos", "center center").is_ok());
331+
assert!(validate_image_position("pos", "center bottom").is_ok());
332+
assert!(validate_image_position("pos", "left top").is_err());
333+
assert!(validate_image_position("pos", "expression(alert(1))").is_err());
334+
assert!(validate_image_position("pos", "").is_err());
335+
}
336+
289337
#[test]
290338
fn test_is_private_ip() {
291339
assert!(is_private_ip(&"127.0.0.1".parse().unwrap()));

0 commit comments

Comments
 (0)