Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1.x.x - HBParser harden #411

Merged
merged 4 commits into from Mar 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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