From d2ff9d2617d099fff64e999233e9fb3fbf70b9f6 Mon Sep 17 00:00:00 2001
From: ergosteur <ergosteur@1qaz.ca>
Date: Fri, 21 Aug 2026 17:13:23 -0400
Subject: [PATCH] Fix stored-HTML-injection: escape AD data before it hits
 Markdown tables

Every table cell fed from directory content (descriptions, sAMAccountName,
OS strings, DNs, object classes) was interpolated into the Markdown report
raw. python-markdown doesn't escape inline HTML by default, so a
directory-controlled value like an OU description containing <script>...
would render live once the report was converted to HTML with md_to_html.py
-- and the underlying data is attacker-influenceable, not just
operator-authored.

Added md_escape() (Python) / ConvertTo-MdSafe (PowerShell), applied at
every table-row interpolation in both scripts. Escapes &, <, > to HTML
entities and | plus embedded newlines to keep the table structure intact.
Raw JSON dumps are left untouched -- this only affects the Markdown/HTML
presentation layer.

Verified end-to-end with <script>, <img onerror=...>, embedded &, and
embedded | payloads across every affected table in both scripts; confirmed
no live tags reach the rendered HTML and no double-escaping occurs.
---
 src/Invoke-ADAudit.ps1 | 33 +++++++++++++++++++++++++--------
 src/ad_audit.py        | 37 +++++++++++++++++++++++++++++--------
 2 files changed, 54 insertions(+), 16 deletions(-)

diff --git a/src/Invoke-ADAudit.ps1 b/src/Invoke-ADAudit.ps1
index 7c0988d..97e983e 100644
--- a/src/Invoke-ADAudit.ps1
+++ b/src/Invoke-ADAudit.ps1
@@ -79,6 +79,23 @@ function ConvertTo-Rfc3339($DateTime) {
     return $DateTime.ToUniversalTime().ToString("yyyy-MM-ddTHH:mm:ssZ")
 }
 
+# AD attributes (descriptions, sAMAccountName, OS strings, DNs, ...) are
+# directory content, not report-generated text -- they can contain anything
+# a writer or an attacker put there, including raw HTML. Markdown doesn't
+# escape inline HTML by default, so an unescaped '<script>' in an OU
+# description would render live when the report is viewed as HTML. This also
+# neutralizes '|' and embedded newlines, which would otherwise corrupt the
+# table row itself. Call on every AD-sourced value before it goes into a
+# Markdown table cell.
+function ConvertTo-MdSafe($Value) {
+    if ($null -eq $Value) { return "" }
+    $text = [string]$Value
+    $text = $text.Replace("&", "&amp;").Replace("<", "&lt;").Replace(">", "&gt;")
+    $text = $text.Replace("|", "\|")
+    $text = ($text -replace "\s*[\r\n]+\s*", " ").Trim()
+    return $text
+}
+
 # ---------------------------------------------------------------------------
 # Object type counts (whole subtree)
 # ---------------------------------------------------------------------------
@@ -301,7 +318,7 @@ if ($findings.Count -gt 0) {
     Add-Line ""
     Add-Line "| Severity | Finding | Count | Notes |"
     Add-Line "|---|---|---|---|"
-    foreach ($f in $findings) { Add-Line "| $($f.Severity) | $($f.Title) | $($f.Rows.Count) | $($f.Notes) |" }
+    foreach ($f in $findings) { Add-Line "| $($f.Severity) | $(ConvertTo-MdSafe $f.Title) | $($f.Rows.Count) | $(ConvertTo-MdSafe $f.Notes) |" }
     Add-Line ""
 
     Add-Line "### Finding Detail Lists"
@@ -309,9 +326,9 @@ if ($findings.Count -gt 0) {
     foreach ($f in $findings) {
         Add-Line "#### [$($f.Severity)] $($f.Title)"
         Add-Line ""
-        Add-Line "| $($f.Cols[0]) | $($f.Cols[1]) | $($f.Cols[2]) |"
+        Add-Line "| $(ConvertTo-MdSafe $f.Cols[0]) | $(ConvertTo-MdSafe $f.Cols[1]) | $(ConvertTo-MdSafe $f.Cols[2]) |"
         Add-Line "|---|---|---|"
-        foreach ($row in $f.Rows) { Add-Line "| $($row.Col0) | $($row.Col1) | $($row.DN) |" }
+        foreach ($row in $f.Rows) { Add-Line "| $(ConvertTo-MdSafe $row.Col0) | $(ConvertTo-MdSafe $row.Col1) | $(ConvertTo-MdSafe $row.DN) |" }
         Add-Line ""
     }
 } else {
@@ -323,7 +340,7 @@ Add-Line "## Object Type Counts"
 Add-Line ""
 Add-Line "| Object Class | Count |"
 Add-Line "|---|---|"
-foreach ($row in $objectCounts) { Add-Line "| $($row.ObjectClass) | $($row.Count) |" }
+foreach ($row in $objectCounts) { Add-Line "| $(ConvertTo-MdSafe $row.ObjectClass) | $($row.Count) |" }
 Add-Line ""
 
 Add-Line "## Organizational Units"
@@ -333,7 +350,7 @@ Add-Line "- Maximum nesting depth: $maxDepth"
 Add-Line ""
 Add-Line "| OU DN | Depth | Description |"
 Add-Line "|---|---|---|"
-foreach ($o in ($ous | Sort-Object DN)) { Add-Line "| $($o.DN) | $($o.Depth) | $($o.Description) |" }
+foreach ($o in ($ous | Sort-Object DN)) { Add-Line "| $(ConvertTo-MdSafe $o.DN) | $($o.Depth) | $(ConvertTo-MdSafe $o.Description) |" }
 Add-Line ""
 
 Add-Line "## Users"
@@ -362,7 +379,7 @@ Add-Line ""
 Add-Line "| Operating System | Count |"
 Add-Line "|---|---|"
 $osGroups = $computers | Group-Object -Property { if ($_.OS) { $_.OS } else { "Unknown" } } | Sort-Object Count -Descending
-foreach ($g in $osGroups) { Add-Line "| $($g.Name) | $($g.Count) |" }
+foreach ($g in $osGroups) { Add-Line "| $(ConvertTo-MdSafe $g.Name) | $($g.Count) |" }
 Add-Line ""
 
 Add-Line "## Groups"
@@ -375,7 +392,7 @@ Add-Line ""
 Add-Line "| Scope | Count |"
 Add-Line "|---|---|"
 $scopeGroups = $groups | Group-Object -Property Scope | Sort-Object Count -Descending
-foreach ($g in $scopeGroups) { Add-Line "| $($g.Name) | $($g.Count) |" }
+foreach ($g in $scopeGroups) { Add-Line "| $(ConvertTo-MdSafe $g.Name) | $($g.Count) |" }
 Add-Line ""
 
 Add-Line "### Largest groups (top 15 by member count)"
@@ -383,7 +400,7 @@ Add-Line ""
 Add-Line "| Group | Type | Scope | Members |"
 Add-Line "|---|---|---|---|"
 foreach ($g in ($groups | Sort-Object -Property MemberCount -Descending | Select-Object -First 15)) {
-    Add-Line "| $($g.SamAccountName) | $($g.Type) | $($g.Scope) | $($g.MemberCount) |"
+    Add-Line "| $(ConvertTo-MdSafe $g.SamAccountName) | $(ConvertTo-MdSafe $g.Type) | $(ConvertTo-MdSafe $g.Scope) | $($g.MemberCount) |"
 }
 Add-Line ""
 
diff --git a/src/ad_audit.py b/src/ad_audit.py
index d08bd0e..097dfb2 100644
--- a/src/ad_audit.py
+++ b/src/ad_audit.py
@@ -11,6 +11,7 @@ import argparse
 import datetime
 import getpass
 import json
+import re
 import sys
 from collections import Counter, defaultdict
 
@@ -43,6 +44,26 @@ def filetime_to_datetime(value):
         return None
 
 
+def md_escape(value):
+    """Escape a value before it goes into a Markdown table cell.
+
+    AD attributes (descriptions, sAMAccountName, OS strings, DNs, ...) are
+    directory content, not report-generated text -- they can contain
+    anything a writer or an attacker put there, including raw HTML. Markdown
+    doesn't escape inline HTML by default, so an unescaped '<script>' in an
+    OU description would render live when the report is viewed as HTML.
+    Also neutralizes '|' and embedded newlines, which would otherwise
+    corrupt the table row itself.
+    """
+    if value is None:
+        return ""
+    text = str(value)
+    text = text.replace("&", "&amp;").replace("<", "&lt;").replace(">", "&gt;")
+    text = text.replace("|", "\\|")
+    text = re.sub(r"\s*[\r\n]+\s*", " ", text).strip()
+    return text
+
+
 def rfc3339(dt):
     """Format a datetime as RFC 3339 (UTC, second precision, 'Z' suffix).
 
@@ -408,7 +429,7 @@ def build_executive_summary(data, stats, args):
         a("| Severity | Finding | Count | Notes |")
         a("|---|---|---|---|")
         for severity, title, notes, cols, rows in findings:
-            a(f"| {severity} | {title} | {len(rows)} | {notes} |")
+            a(f"| {severity} | {md_escape(title)} | {len(rows)} | {md_escape(notes)} |")
         a("")
 
         a("### Finding Detail Lists")
@@ -416,10 +437,10 @@ def build_executive_summary(data, stats, args):
         for severity, title, notes, cols, rows in findings:
             a(f"#### [{severity}] {title}")
             a("")
-            a(f"| {cols[0]} | {cols[1]} | {cols[2]} |")
+            a(f"| {md_escape(cols[0])} | {md_escape(cols[1])} | {md_escape(cols[2])} |")
             a("|---|---|---|")
             for col0, col1, dn in rows:
-                a(f"| {col0} | {col1} | {dn} |")
+                a(f"| {md_escape(col0)} | {md_escape(col1)} | {md_escape(dn)} |")
             a("")
     else:
         a("No notable risk or cleanup findings surfaced by this audit's checks.")
@@ -454,7 +475,7 @@ def build_report(data, args):
     a("| Object Class | Count |")
     a("|---|---|")
     for cls, count in sorted(obj_counts.items(), key=lambda kv: -kv[1]):
-        a(f"| {cls} | {count} |")
+        a(f"| {md_escape(cls)} | {count} |")
     a("")
 
     a("## Organizational Units")
@@ -465,7 +486,7 @@ def build_report(data, args):
     a("| OU DN | Depth | Description |")
     a("|---|---|---|")
     for o in sorted(ous, key=lambda x: x["dn"]):
-        a(f"| {o['dn']} | {o['depth']} | {o['description']} |")
+        a(f"| {md_escape(o['dn'])} | {o['depth']} | {md_escape(o['description'])} |")
     a("")
 
     a("## Users")
@@ -509,7 +530,7 @@ def build_report(data, args):
     a("| Operating System | Count |")
     a("|---|---|")
     for os_name, count in sorted(os_counter.items(), key=lambda kv: -kv[1]):
-        a(f"| {os_name} | {count} |")
+        a(f"| {md_escape(os_name)} | {count} |")
     a("")
 
     a("## Groups")
@@ -527,7 +548,7 @@ def build_report(data, args):
     a("| Scope | Count |")
     a("|---|---|")
     for scope, count in scope_counter.most_common():
-        a(f"| {scope} | {count} |")
+        a(f"| {md_escape(scope)} | {count} |")
     a("")
 
     largest = sorted(groups, key=lambda g: -g["member_count"])[:15]
@@ -536,7 +557,7 @@ def build_report(data, args):
     a("| Group | Type | Scope | Members |")
     a("|---|---|---|---|")
     for g in largest:
-        a(f"| {g['sam']} | {g['type']} | {g['scope']} | {g['member_count']} |")
+        a(f"| {md_escape(g['sam'])} | {md_escape(g['type'])} | {md_escape(g['scope'])} | {g['member_count']} |")
     a("")
 
     return "\n".join(lines)