diff --git a/common/models/shipping/Ups.php b/common/models/shipping/Ups.php
index 28f6340f3..053f62d9e 100644
--- a/common/models/shipping/Ups.php
+++ b/common/models/shipping/Ups.php
@@ -233,11 +233,25 @@ function getQuote($address)
// $fetch_accountrates = ($this->config->get('shipping_hitups_auto_rate_type') == 'ACCOUNT') ? "" . $custom_settings[$key]['ups_acc_no'] . "" : "";
+ $ups_access_key = $this->escapeXmlValue($custom_settings[$key]['ups_access_key']);
+ $ups_id = $this->escapeXmlValue($custom_settings[$key]['ups_id']);
+ $ups_pass = $this->escapeXmlValue($custom_settings[$key]['ups_pass']);
+ $ups_account = $this->escapeXmlValue($custom_settings[$key]['ups_acc_no']);
+ $shipper_name = $this->escapeXmlValue($custom_settings[$key]['ups_ship_name']);
+ $ship_city = $this->escapeXmlValue($custom_settings[$key]['ups_ship_city']);
+ $ship_state = $this->escapeXmlValue($custom_settings[$key]['ups_ship_state']);
+ $ship_country = $this->escapeXmlValue($custom_settings[$key]['ups_ship_country']);
+ $ship_postcode = $this->escapeXmlValue($custom_settings[$key]['ups_ship_zip']);
+ $destination_city = $this->escapeXmlValue($address['city']);
+ $destination_state = $this->escapeXmlValue($address['zone_code']);
+ $destination_country = $this->escapeXmlValue($address['iso_code_2']);
+ $destination_postcode = $this->escapeXmlValue($address['postcode']);
+
$xml = '';
$xml .= '';
- $xml .= ' ' . $custom_settings[$key]['ups_access_key'] . '';
- $xml .= ' ' . $custom_settings[$key]['ups_id'] . '';
- $xml .= ' ' . $custom_settings[$key]['ups_pass'] . '';
+ $xml .= ' ' . $ups_access_key . '';
+ $xml .= ' ' . $ups_id . '';
+ $xml .= ' ' . $ups_pass . '';
$xml .= '';
$xml .= '';
$xml .= '';
@@ -251,29 +265,29 @@ function getQuote($address)
$xml .= ' ';
$xml .= ' ';
$xml .= ' ';
- $xml .= ' ' . $custom_settings[$key]['ups_ship_name'] . '';
- $xml .= ' ' . $custom_settings[$key]['ups_acc_no'] . '';
+ $xml .= ' ' . $shipper_name . '';
+ $xml .= ' ' . $ups_account . '';
$xml .= ' ';
- $xml .= ' ' . $custom_settings[$key]['ups_ship_city'] . '';
- $xml .= ' ' . $custom_settings[$key]['ups_ship_state'] . '';
- $xml .= ' ' . $custom_settings[$key]['ups_ship_country'] . '';
- $xml .= ' ' . $custom_settings[$key]['ups_ship_zip'] . '';
+ $xml .= ' ' . $ship_city . '';
+ $xml .= ' ' . $ship_state . '';
+ $xml .= ' ' . $ship_country . '';
+ $xml .= ' ' . $ship_postcode . '';
$xml .= ' ';
$xml .= ' ';
$xml .= ' ';
$xml .= ' ';
- $xml .= ' ' . $address['city'] . '';
- $xml .= ' ' . $address['zone_code'] . '';
- $xml .= ' ' . $address['iso_code_2'] . '';
- $xml .= ' ' . $address['postcode'] . '';
+ $xml .= ' ' . $destination_city . '';
+ $xml .= ' ' . $destination_state . '';
+ $xml .= ' ' . $destination_country . '';
+ $xml .= ' ' . $destination_postcode . '';
$xml .= ' ';
$xml .= ' ';
$xml .= ' ';
$xml .= ' ';
- $xml .= ' ' . $custom_settings[$key]['ups_ship_city'] . '';
- $xml .= ' ' . $custom_settings[$key]['ups_ship_state'] . '';
- $xml .= ' ' . $custom_settings[$key]['ups_ship_country'] . '';
- $xml .= ' ' . $custom_settings[$key]['ups_ship_zip'] . '';
+ $xml .= ' ' . $ship_city . '';
+ $xml .= ' ' . $ship_state . '';
+ $xml .= ' ' . $ship_country . '';
+ $xml .= ' ' . $ship_postcode . '';
$xml .= ' ';
$xml .= ' ';
$xml .= $packages_xml;
@@ -392,6 +406,11 @@ function getQuote($address)
return $method_data;
}
+ private function escapeXmlValue($value)
+ {
+ return htmlspecialchars((string) $value, ENT_XML1 | ENT_QUOTES, 'UTF-8');
+ }
+
public function getDefaultVenConfig($vendor_id = "default"){
$custom_settings['ups_id'] = $this->config->get('shipping_hitups_auto_key');
$custom_settings['ups_pass'] = $this->config->get('shipping_hitups_auto_password');
@@ -409,4 +428,4 @@ public function getDefaultVenConfig($vendor_id = "default"){
$custom_settings['ups_ship_country'] = $this->config->get('shipping_hitups_auto_country_code');
return $custom_settings;
}
-}
\ No newline at end of file
+}
diff --git a/tests/check-ups-rate-xml-escaping.py b/tests/check-ups-rate-xml-escaping.py
new file mode 100644
index 000000000..d9123a570
--- /dev/null
+++ b/tests/check-ups-rate-xml-escaping.py
@@ -0,0 +1,33 @@
+from pathlib import Path
+
+
+ups = Path("common/models/shipping/Ups.php")
+source = ups.read_text(encoding="utf-8")
+
+required_snippets = [
+ "private function escapeXmlValue($value)",
+ "htmlspecialchars((string) $value, ENT_XML1 | ENT_QUOTES, 'UTF-8')",
+ "$ups_access_key = $this->escapeXmlValue($custom_settings[$key]['ups_access_key']);",
+ "$ship_city = $this->escapeXmlValue($custom_settings[$key]['ups_ship_city']);",
+ "$destination_city = $this->escapeXmlValue($address['city']);",
+ "'\t' . $ups_access_key . ''",
+ "'\t\t' . $shipper_name . ''",
+ "'\t\t\t\t' . $destination_city . ''",
+]
+
+missing = [snippet for snippet in required_snippets if snippet not in source]
+if missing:
+ raise SystemExit(
+ "UPS rate XML is missing expected escaping snippets: " + ", ".join(missing)
+ )
+
+unsafe_snippets = [
+ "'\t' . $custom_settings[$key]['ups_access_key']",
+ "'\t\t' . $custom_settings[$key]['ups_ship_name']",
+ "'\t\t\t\t' . $custom_settings[$key]['ups_ship_city']",
+ "' \t\t\t\t' . $address['city']",
+]
+
+for unsafe_snippet in unsafe_snippets:
+ if unsafe_snippet in source:
+ raise SystemExit(f"UPS rate XML still writes raw value: {unsafe_snippet}")