Skip to content

Commit

Permalink
1.x.x - HBParser harden (#411)
Browse files Browse the repository at this point in the history
  • Loading branch information
adam-fowler committed Mar 25, 2024
1 parent b67a8db commit 94820ea
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 11 deletions.
8 changes: 2 additions & 6 deletions .github/workflows/ci.yml
Expand Up @@ -4,15 +4,11 @@ on:
push:
branches:
- main
- 1.x.x
paths:
- '**.swift'
- '**.yml'
pull_request:
branches:
- main
paths:
- '**.swift'
- '**.yml'
workflow_dispatch:

jobs:
Expand All @@ -39,9 +35,9 @@ jobs:
strategy:
matrix:
image:
- 'swift:5.7'
- 'swift:5.8'
- 'swift:5.9'
- 'swift:5.10'

container:
image: ${{ matrix.image }}
Expand Down
2 changes: 0 additions & 2 deletions .github/workflows/validate.yml
Expand Up @@ -2,8 +2,6 @@ name: Validity Check

on:
pull_request:
branches:
- main

jobs:
validate:
Expand Down
9 changes: 6 additions & 3 deletions Sources/Hummingbird/Utils/HBParser.swift
Expand Up @@ -596,8 +596,7 @@ extension HBParser {
func _percentDecode(_ original: ArraySlice<UInt8>, _ bytes: UnsafeMutableBufferPointer<UInt8>) 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 {
let high = Self.asciiHexValues[Int(original[index + 1])]
Expand All @@ -614,9 +613,13 @@ extension HBParser {
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, *) {
Expand Down
24 changes: 24 additions & 0 deletions Tests/HummingbirdTests/RouterTests.swift
Expand Up @@ -393,6 +393,30 @@ final class RouterTests: XCTestCase {
XCTAssertEqual(response.status, .seeOther)
}
}

/// 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() throws {
let app = HBApplication(testing: .embedded)
app.router.get("overflow") { req in
let currentQP = req.uri.queryParameters["query"]
return String("\(currentQP ?? "")")
}
try app.XCTStart()
defer { app.XCTStop() }

try app.XCTExecute(uri: "/overflow?query=value%", method: .GET) { response in
let body = try XCTUnwrap(response.body)
XCTAssertEqual(String(buffer: body), "value%")
}
try app.XCTExecute(uri: "/overflow?query%=value%", method: .GET) { response in
let body = try XCTUnwrap(response.body)
XCTAssertEqual(String(buffer: body), "")
}
try app.XCTExecute(uri: "/overflow?%&", method: .GET) { response in
let body = try XCTUnwrap(response.body)
XCTAssertEqual(String(buffer: body), "")
}
}
}

extension HBRequest {
Expand Down

0 comments on commit 94820ea

Please sign in to comment.