From 08131bd4c0ac085dfbdb0254b50f8fb9d7ec3fb0 Mon Sep 17 00:00:00 2001 From: tannerdsilva Date: Mon, 18 Mar 2024 10:30:17 -0500 Subject: [PATCH 1/4] initial hardening - guarding for index overflows --- Sources/HummingbirdCore/Utils/HBParser.swift | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Sources/HummingbirdCore/Utils/HBParser.swift b/Sources/HummingbirdCore/Utils/HBParser.swift index fdf6b67aa..89ae55b9d 100644 --- a/Sources/HummingbirdCore/Utils/HBParser.swift +++ b/Sources/HummingbirdCore/Utils/HBParser.swift @@ -600,6 +600,9 @@ extension Parser { while index < original.endIndex { // if we have found a percent sign if original[index] == 0x25 { + guard original.endIndex - index >= 2 else { + throw DecodeError() + } let high = Self.asciiHexValues[Int(original[index + 1])] let low = Self.asciiHexValues[Int(original[index + 2])] index += 3 From 2d14c7ea434393f6c9a7a86ac051b5d191e12034 Mon Sep 17 00:00:00 2001 From: tannerdsilva Date: Mon, 18 Mar 2024 19:15:39 -0500 Subject: [PATCH 2/4] optimized overflow logic --- Sources/HummingbirdCore/Utils/HBParser.swift | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Sources/HummingbirdCore/Utils/HBParser.swift b/Sources/HummingbirdCore/Utils/HBParser.swift index 89ae55b9d..b09342e6d 100644 --- a/Sources/HummingbirdCore/Utils/HBParser.swift +++ b/Sources/HummingbirdCore/Utils/HBParser.swift @@ -596,13 +596,9 @@ extension Parser { func _percentDecode(_ original: ArraySlice, _ bytes: UnsafeMutableBufferPointer) throws -> Int { var newIndex = 0 var index = original.startIndex - - while index < original.endIndex { + while index < (original.endIndex - 2) { // if we have found a percent sign if original[index] == 0x25 { - guard original.endIndex - index >= 2 else { - throw DecodeError() - } let high = Self.asciiHexValues[Int(original[index + 1])] let low = Self.asciiHexValues[Int(original[index + 2])] index += 3 @@ -617,9 +613,13 @@ extension Parser { index += 1 } } + while index < original.endIndex { + bytes[newIndex] = original[index] + newIndex += 1 + index += 1 + } return newIndex } - guard self.index != self.range.endIndex else { return "" } do { if #available(macOS 11, macCatalyst 14.0, iOS 14.0, tvOS 14.0, *) { From 06828e2bce8caaac6ca510c7ebc87acf3888db74 Mon Sep 17 00:00:00 2001 From: tannerdsilva Date: Tue, 19 Mar 2024 15:55:22 -0500 Subject: [PATCH 3/4] add testcase for percent-encoding overflow and other overflows in similar vein --- .../HummingbirdRouterTests/RouterTests.swift | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/Tests/HummingbirdRouterTests/RouterTests.swift b/Tests/HummingbirdRouterTests/RouterTests.swift index 20859f766..ccdbf01a5 100644 --- a/Tests/HummingbirdRouterTests/RouterTests.swift +++ b/Tests/HummingbirdRouterTests/RouterTests.swift @@ -309,6 +309,28 @@ final class RouterTests: XCTestCase { } } } + + /// Test the hummingbird core parser against possible overflows of the percent encoder. this issue was introduced in pr #404 in the context of query parameters but I've thrown in some other random overflow scenarios in here too for good measure. if it doesn't crash, its a win. + func testQueryParameterOverflow() async throws { + let router = RouterBuilder(context: BasicRouterRequestContext.self) { + Get("overflow") { req, _ in + let currentQP = req.uri.queryParameters["query"] + return String("\(currentQP ?? "")") + } + } + let app = Application(router:router) + try await app.test(.router) { client in + try await client.execute(uri: "/overflow?query=value%", method: .get) { response in + XCTAssertEqual(String(buffer: response.body), "value%") + } + try await client.execute(uri: "/overflow?query%=value%", method: .get) { response in + XCTAssertEqual(String(buffer: response.body), "") + } + try await client.execute(uri: "/overflow?%&", method: .get) { response in + XCTAssertEqual(String(buffer: response.body), "") + } + } + } func testParameters() async throws { let router = RouterBuilder(context: BasicRouterRequestContext.self) { From ecbd7a8eca41fd2f74568ffddc6330c91a19f39c Mon Sep 17 00:00:00 2001 From: tannerdsilva Date: Wed, 20 Mar 2024 09:21:29 -0500 Subject: [PATCH 4/4] resolved inconsistent formatting --- Tests/HummingbirdRouterTests/RouterTests.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Tests/HummingbirdRouterTests/RouterTests.swift b/Tests/HummingbirdRouterTests/RouterTests.swift index ccdbf01a5..70b1ffdfc 100644 --- a/Tests/HummingbirdRouterTests/RouterTests.swift +++ b/Tests/HummingbirdRouterTests/RouterTests.swift @@ -309,7 +309,7 @@ final class RouterTests: XCTestCase { } } } - + /// Test the hummingbird core parser against possible overflows of the percent encoder. this issue was introduced in pr #404 in the context of query parameters but I've thrown in some other random overflow scenarios in here too for good measure. if it doesn't crash, its a win. func testQueryParameterOverflow() async throws { let router = RouterBuilder(context: BasicRouterRequestContext.self) { @@ -318,7 +318,7 @@ final class RouterTests: XCTestCase { return String("\(currentQP ?? "")") } } - let app = Application(router:router) + let app = Application(router: router) try await app.test(.router) { client in try await client.execute(uri: "/overflow?query=value%", method: .get) { response in XCTAssertEqual(String(buffer: response.body), "value%") @@ -327,7 +327,7 @@ final class RouterTests: XCTestCase { XCTAssertEqual(String(buffer: response.body), "") } try await client.execute(uri: "/overflow?%&", method: .get) { response in - XCTAssertEqual(String(buffer: response.body), "") + XCTAssertEqual(String(buffer: response.body), "") } } }