Harden the HTML output (#141)

* Escape characters to prevent xss sinks * Escape the value of html attributes * Differentiate the character escaping of an attribute and an element body * Secure the components as well * Add the option for the autoescaping to the vapor provider With the option, people can decide if they want it or not.
vapor-community · Sep 4, 2023 · 676792d · 676792d
1 parent 5990bb3
commit 676792d
Show file tree

Hide file tree

Showing 11 changed files with 320 additions and 42 deletions.
diff --git a/Sources/HTMLKit/Framework/Rendering/Renderer.swift b/Sources/HTMLKit/Framework/Rendering/Renderer.swift
@@ -42,19 +42,31 @@ public class Renderer {
     private var environment: Environment
 
     private var localization: Localization?
+
+    private var security: Security
 
     /// Initiates the renderer.
     public init(localization: Localization? = nil) {
 
+        self.localization = localization
         self.environment = Environment()
+        self.security = Security()
+    }
+
+    /// Initiates the renderer.
+    public init(localization: Localization? = nil, security: Security) {
+
         self.localization = localization
+        self.environment = Environment()
+        self.security = security
     }
 
     /// Initiates the renderer.
-    public init(localization: Localization? = nil, environment: Environment) {
+    public init(localization: Localization? = nil, environment: Environment, security: Security) {
 
-        self.environment = environment
         self.localization = localization
+        self.environment = environment
+        self.security = security
     }
 
     /// Renders a view
@@ -112,11 +124,11 @@ public class Renderer {
             }
 
             if let value = content as? EnvironmentValue {
-                result += try render(value: value)
+                result += escape(content: try render(value: value))
             }
 
             if let element = content as? String {
-                result += element
+                result += escape(content: (element))
             }
         }
 
@@ -177,11 +189,11 @@ public class Renderer {
                 }
 
                 if let value = content as? EnvironmentValue {
-                    result += try render(value: value)
+                    result += escape(content: try render(value: value))
                 }
 
                 if let element = content as? String {
-                    result += element
+                    result += escape(content: element)
                 }
             }
         }
@@ -209,12 +221,30 @@ public class Renderer {
 
     /// Renders a document element
     internal func render(element: some DocumentNode) -> String {
-        return "<!DOCTYPE \(element.content)>"
+
+        var result = ""
+
+        result += "<!DOCTYPE "
+
+        result += element.content
+
+        result += ">"
+
+        return result
     }
 
     /// Renders a comment element
     internal func render(element: some CommentNode) -> String {
-        return "<!--\(element.content)-->"
+
+        var result = ""
+
+        result += "<!--"
+
+        result += escape(content: element.content)
+
+        result += "-->"
+
+        return result
     }
 
     /// Renders a content element
@@ -266,8 +296,12 @@ public class Renderer {
                     result += try render(stringkey: stringkey)
                 }
 
+                if let value = content as? EnvironmentValue {
+                    result += escape(content: try render(value: value))
+                }
+
                 if let element = content as? String {
-                    result += element
+                    result += escape(content: element)
                 }
             }
         }
@@ -348,7 +382,7 @@ public class Renderer {
             return String(doubleValue)
 
         case let stringValue as String:
-            return String(stringValue)
+            return stringValue
 
         case let dateValue as Date:
 
@@ -374,13 +408,38 @@ public class Renderer {
             result += " \(attribute.key)=\""
 
             if let value = attribute.value as? EnvironmentValue {
-                result += "\(try render(value: value))\""
+                result += escape(attribute: try render(value: value))
 
             } else {
-                result += "\(attribute.value)\""
+                result += escape(attribute: "\(attribute.value)")
             }
+
+            result += "\""
         }
 
         return result
     }
+
+    /// Converts specific charaters into encoded values.
+    internal func escape(attribute value: String) -> String {
+
+        if security.autoEscaping {
+            return value.replacingOccurrences(of: "&", with: "&amp;")
+                .replacingOccurrences(of: "\"", with: "&quot;")
+                .replacingOccurrences(of: "'", with: "&apos;")
+        }
+
+        return value
+    }
+
+    /// Converts specific charaters into encoded values.
+    internal func escape(content value: String) -> String {
+
+        if security.autoEscaping {
+            return value.replacingOccurrences(of: "<", with: "&lt;")
+                .replacingOccurrences(of: ">", with: "&gt;")
+        }
+
+        return value
+    }
 }
diff --git a/Sources/HTMLKit/Framework/Security/Security.swift b/Sources/HTMLKit/Framework/Security/Security.swift
@@ -0,0 +1,9 @@
+public class Security {
+
+    public var autoEscaping: Bool
+
+    public init() {
+
+        self.autoEscaping = true
+    }
+}
diff --git a/Sources/HTMLKitComponents/Actions/SubmitAction.swift b/Sources/HTMLKitComponents/Actions/SubmitAction.swift
@@ -13,10 +13,10 @@ extension SubmitAction {
         if let data = try? JSONEncoder().encode(validators) {
 
             if let result = String(data: data, encoding: .utf8) {
-                return "$('#\(target)').validate('\(result)');"
+                return "$('#\(target.escape())').validate('\(result)');"
             }
         }
 
-        return "$('#\(target)').validate('[]');"
+        return "$('#\(target.escape())').validate('[]');"
     }
 }
diff --git a/Sources/HTMLKitComponents/Actions/ViewAction.swift b/Sources/HTMLKitComponents/Actions/ViewAction.swift
@@ -21,22 +21,22 @@ public protocol ViewAction {
 extension ViewAction {
 
     public func show(_ target: String) -> String {
-        return "$('#\(target)').show();"
+        return "$('#\(target.escape())').show();"
     }
 
     public func hide(_ target: String) -> String {
-        return "$('#\(target)').hide();"
+        return "$('#\(target.escape())').hide();"
     }
 
     public func animate(_ target: String) -> String {
-        return "$('#\(target)').animate();"
+        return "$('#\(target.escape())').animate();"
     }
 
     public func open(_ target: String) -> String {
-        return "$('#\(target)').open();"
+        return "$('#\(target.escape())').open();"
     }
 
     public func close(_ target: String) -> String {
-        return "$('#\(target)').close();"
+        return "$('#\(target.escape())').close();"
     }
 }
diff --git a/Sources/HTMLKitComponents/Extensions/Components+String.swift b/Sources/HTMLKitComponents/Extensions/Components+String.swift
@@ -0,0 +1,11 @@
+extension String {
+
+    internal func escape() -> String {
+
+        return self.replacingOccurrences(of: "&", with: "&amp;")
+            .replacingOccurrences(of: "<", with: "&lt;")
+            .replacingOccurrences(of: ">", with: "&gt;")
+            .replacingOccurrences(of: "\"", with: "&quot;")
+            .replacingOccurrences(of: "'", with: "&apos;")
+    }
+}
diff --git a/Sources/HTMLKitVapor/Extensions/Vapor+HTMLKit.swift b/Sources/HTMLKitVapor/Extensions/Vapor+HTMLKit.swift
@@ -26,6 +26,24 @@ extension Application {
             public typealias Value = HTMLKit.Environment
         }
 
+        internal struct SecurityStorageKey: StorageKey {
+
+            public typealias Value = HTMLKit.Security
+        }
+
+        public var security: HTMLKit.Security {
+
+            if let configuration = self.application.storage[SecurityStorageKey.self] {
+                return configuration
+            }
+
+            let configuration = Security()
+
+            self.application.storage[SecurityStorageKey.self] = configuration
+
+            return configuration
+        }
+
         /// The view localization
         public var localization: HTMLKit.Localization {
 
@@ -59,7 +77,8 @@ extension Application {
 
             return .init(eventLoop: application.eventLoopGroup.next(),
                          localization: localization,
-                         environment: environment)
+                         environment: environment,
+                         security: security)
         }
 
         /// The application dependency
@@ -94,6 +113,7 @@ extension Request {
 
         return .init(eventLoop: self.eventLoop,
                      localization: self.application.htmlkit.localization,
-                     environment: self.application.htmlkit.environment)
+                     environment: self.application.htmlkit.environment,
+                     security: self.application.htmlkit.security)
     }
 }
diff --git a/Sources/HTMLKitVapor/ViewRenderer.swift b/Sources/HTMLKitVapor/ViewRenderer.swift
@@ -16,10 +16,10 @@ public class ViewRenderer {
     internal var renderer: Renderer
 
     /// Creates the view renderer
-    public init(eventLoop: EventLoop, localization: HTMLKit.Localization, environment: HTMLKit.Environment) {
+    public init(eventLoop: EventLoop, localization: Localization, environment: HTMLKit.Environment, security: Security) {
 
         self.eventLoop = eventLoop
-        self.renderer = Renderer(localization: localization, environment: environment)
+        self.renderer = Renderer(localization: localization, environment: environment, security: security)
     }
 
     /// Renders a layout and its context

diff --git a/Tests/HTMLKitComponentsTests/SecurityTests.swift b/Tests/HTMLKitComponentsTests/SecurityTests.swift
@@ -0,0 +1,77 @@
+import HTMLKit
+import HTMLKitComponents
+import XCTest
+
+final class SecurityTests: XCTestCase {
+
+    struct TestView: View {
+
+        @ContentBuilder<Content> var body: Content
+    }
+
+    var renderer = Renderer()
+
+    func testEncodingAttributeContext() throws {
+
+        let attack = "\" onclick=\"alert(1);\""
+
+        let view = TestView {
+            Button(role: .button) {
+                "Show"
+            }
+            .tag(attack)
+        }
+
+        XCTAssertEqual(try renderer.render(view: view),
+                       """
+                       <button type="button" class="button" id="&quot; onclick=&quot;alert(1);&quot;">Show</button>
+                       """
+        )
+    }
+
+    func testEncodingActionContext() throws {
+
+        let attack = "test').appendTo(''); $('#test"
+
+        let view = TestView {
+            Button(role: .button) {
+                "Show"
+            }
+            .tag("sender")
+            .onClick { button in
+                button.show(attack)
+            }
+
+        }
+
+        XCTAssertEqual(try renderer.render(view: view),
+                       """
+                       <button type="button" class="button" id="sender">Show</button>\
+                       <script>\
+                       $('#sender').onClick(function(){\
+                       $('#test&apos;).appendTo(&apos;&apos;); $(&apos;#test').show();\
+                       });\
+                       </script>
+                       """
+        )
+    }
+
+    func testEncodingCssContext() throws {
+
+        let attack = "test\" style=\"property: unsafe\""
+
+        let view = TestView {
+            Text {
+                "Text"
+            }
+            .backgroundColor(.custom(attack))
+        }
+
+        XCTAssertEqual(try renderer.render(view: view),
+                       """
+                       <p class="text background:test&quot; style=&quot;property: unsafe&quot;">Text</p>
+                       """
+        )
+    }
+}
+
diff --git a/Tests/HTMLKitTests/PerformanceTests.swift b/Tests/HTMLKitTests/PerformanceTests.swift
@@ -7,13 +7,33 @@ import HTMLKit
 import XCTest
 
 final class PerformanceTests: XCTestCase {
-
-    var renderer = Renderer()
 
-    func testPerformance() throws {
+    func testPerformanceWithoutAutoEscaping() throws {
+
+        let context = SampleContext(id: 0, title: "TestPage", excerpt: "Testpage", modified: Date(), posted: Date())
+
+        let security = Security()
+        security.autoEscaping = false
+
+        let renderer = Renderer(security: security)
+
+        measure {
+
+            for _ in 0...1000 {
+                _ = try! renderer.render(view: SampleView(context: context))
+            }
+        }
+    }
+
+    func testPerformanceWithAutoEscaping() throws {
 
         let context = SampleContext(id: 0, title: "TestPage", excerpt: "Testpage", modified: Date(), posted: Date())
 
+        let security = Security()
+        security.autoEscaping = true
+
+        let renderer = Renderer(security: security)
+
         measure {
 
             for _ in 0...1000 {