From 1fb5886d065f749aeda015d4985c86b46c13e6d6 Mon Sep 17 00:00:00 2001
From: Darko Gjorgjijoski <dg@darkog.com>
Date: Tue, 7 Apr 2026 20:36:05 +0200
Subject: [PATCH] Sanitize PDF address fields against SSRF in
 getFormattedString chokepoint
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Closes the residual surface from the three published SSRF advisories (GHSA-pc5v-8xwc-v9xq, GHSA-38hf-fq8x-q49r, GHSA-q9wx-ggwq-mcgh / CVE-2026-34365 to 34367) that the original 2.2.0 fix only covered for the Notes field. The same blade templates render company/billing/shipping address fields with {!! !!} via Invoice/Estimate/Payment::getCompanyAddress(), getCustomerBillingAddress(), getCustomerShippingAddress() — and those flow through GeneratesPdfTrait::getFormattedString() which did not call PdfHtmlSanitizer.

Customer-controlled fields (name, street, phone, custom-field values) are substituted into address templates via getFieldsArray() without HTML-escaping, so a malicious customer name like "Acme <img src='http://attacker/probe'>" reaches Dompdf as raw HTML through the address path. Today this is blocked only by the secondary defense of dompdf's enable_remote=false; if a self-hoster sets DOMPDF_ENABLE_REMOTE=true for legitimate remote logos, the address surface immediately re-opens.

Move PdfHtmlSanitizer::sanitize() into the chokepoint at GeneratesPdfTrait::getFormattedString() so all four sinks — notes plus the three address fields, on all three models — get the same treatment via a single call site. v3.0's models (Invoice, Estimate, Payment) already had the simpler getNotes() shape (no per-method PdfHtmlSanitizer wrapper), so the trait edit alone is sufficient — no model edits required on this branch. Verified getFormattedString() is only called from PDF code paths (no email body callers, which use strtr() directly).

This is the v3.0 counterpart to master's f387e751. Re-implemented directly on v3.0 instead of cherry-picked because the import-block divergence from the larger v3.0 refactor produced four merge conflicts that were noisier than just porting the chokepoint change manually.

Extends tests/Unit/PdfHtmlSanitizerTest.php with three new cases covering the address-template scenario, iframe/link tag stripping, and on* event handler removal. All 8 tests pass via vendor/bin/pest tests/Unit/PdfHtmlSanitizerTest.php.
---
 app/Traits/GeneratesPdfTrait.php    |  7 +++++-
 tests/Unit/PdfHtmlSanitizerTest.php | 36 +++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/app/Traits/GeneratesPdfTrait.php b/app/Traits/GeneratesPdfTrait.php
index 1953dc11..051e6683 100644
--- a/app/Traits/GeneratesPdfTrait.php
+++ b/app/Traits/GeneratesPdfTrait.php
@@ -7,6 +7,7 @@ use App\Models\CompanySetting;
 use App\Models\FileDisk;
 use App\Models\Setting;
 use App\Services\FontService;
+use App\Support\PdfHtmlSanitizer;
 use Carbon\Carbon;
 use Illuminate\Support\Facades\App;
 
@@ -186,6 +187,10 @@ trait GeneratesPdfTrait
 
         $str = str_replace('</p>', '<br />', $str);
 
-        return $str;
+        // Sanitize the assembled HTML to strip any SSRF vectors that may have
+        // entered through user-supplied address fields, customer names, or
+        // custom field values. Notes also pass through this method, so they
+        // get the same treatment without needing a separate wrapper.
+        return PdfHtmlSanitizer::sanitize($str);
     }
 }
diff --git a/tests/Unit/PdfHtmlSanitizerTest.php b/tests/Unit/PdfHtmlSanitizerTest.php
index d94120f4..12f8f152 100644
--- a/tests/Unit/PdfHtmlSanitizerTest.php
+++ b/tests/Unit/PdfHtmlSanitizerTest.php
@@ -34,3 +34,39 @@ it('normalizes legacy closing-br markup so lines are not collapsed in PDF output
     expect($out)->toContain('<br')->toContain('line1')->toContain('line2');
     expect($out)->not->toBe('line1line2');
 });
+
+it('strips SSRF vectors injected via address-template placeholders', function () {
+    // Simulates the output of GeneratesPdfTrait::getFormattedString() after a
+    // malicious customer name like "Acme <img src='http://attacker/probe'>" has
+    // been substituted into an address template via {BILLING_ADDRESS_NAME}.
+    $html = "Acme <img src='http://attacker.test/probe'><br />123 Main St<br />Springfield";
+
+    $out = PdfHtmlSanitizer::sanitize($html);
+
+    expect($out)->not->toContain('<img');
+    expect($out)->not->toContain('src=');
+    expect($out)->not->toContain('attacker.test');
+    expect($out)->toContain('Acme');
+    expect($out)->toContain('123 Main St');
+    expect($out)->toContain('Springfield');
+});
+
+it('strips iframe and link tags that could trigger SSRF', function () {
+    $html = '<iframe src="http://attacker/x"></iframe><link rel="stylesheet" href="http://attacker/y.css">Hello';
+
+    $out = PdfHtmlSanitizer::sanitize($html);
+
+    expect($out)->not->toContain('<iframe');
+    expect($out)->not->toContain('<link');
+    expect($out)->not->toContain('attacker');
+    expect($out)->toContain('Hello');
+});
+
+it('strips on* event handler attributes from allowed tags', function () {
+    $html = '<p onload="alert(1)" onclick="x">click me</p>';
+
+    $out = PdfHtmlSanitizer::sanitize($html);
+
+    expect($out)->not->toContain('onload')->not->toContain('onclick')->not->toContain('alert');
+    expect($out)->toContain('click me');
+});