2.x.x: guard for index overflow

[tannerdsilva]

hello,

I have been using the hummingbird 2.x.x alphas (and now beta) in production (like a madman) and have been super impressed with how well it has been running. thank you for all your work on this project - its fantastic.

as I expose this socket to more of the wild, I am beginning to pick up on traffic that is causing 2.x.x beta1 and priors to crash.

here is a stack trace of what that looks like (from Ubuntu Linux):

Swift/SliceBuffer.swift:287: Fatal error: Index out of bounds

💣 Program crashed: Illegal instruction at 0x00007f27f34f219e

Thread 1 crashed:

 0 0x00007f27f34f219e closure #1 in closure #1 in closure #1 in _assertionFailure(_:_:file:line:flags:) + 318 in libswiftCore.so
 1 0x00007f27f34f1f4e closure #1 in closure #1 in _assertionFailure(_:_:file:line:flags:) + 269 in libswiftCore.so
 2 0x00007f27f34f1dfd closure #1 in _assertionFailure(_:_:file:line:flags:) + 380 in libswiftCore.so
 3 0x00007f27f34f1986 _assertionFailure(_:_:file:line:flags:) + 229 in libswiftCore.so
 4 0x00007f27f34e7fad ArraySlice.subscript.getter + 108 in libswiftCore.so
 5 _percentDecode #1 (_:_:) in Parser.percentDecode() + 509 in <PROJ_NAME_REDACTED> at <PROJ_ROOT_REDACTED>/.build/checkouts/hummingbird/Sources/HummingbirdCore/Utils/HBParser.swift:603:64
 6 closure #1 in Parser.percentDecode() + 409 in <PROJ_NAME_REDACTED> at <PROJ_ROOT_REDACTED>/.build/checkouts/hummingbird/Sources/HummingbirdCore/Utils/HBParser.swift:624:32
 7 0x00007f27f37459eb specialized static __StringStorage.create(uninitializedCodeUnitCapacity:initializingUncheckedUTF8With:) + 122 in libswiftCore.so
 8 0x00007f27f3745a80 specialized static String._fromLargeUTF8Repairing(uninitializedCapacity:initializingWith:) + 15 in libswiftCore.so
 9 Parser.percentDecode() + 322 in <PROJ_NAME_REDACTED> at <PROJ_ROOT_REDACTED>/.build/checkouts/hummingbird/Sources/HummingbirdCore/Utils/HBParser.swift:623:28
10 closure #1 in URI.queryParameters.getter + 1096 in <PROJ_NAME_REDACTED> at <PROJ_ROOT_REDACTED>/.build/checkouts/hummingbird/Sources/HummingbirdCore/Request/URI.swift:60:64
11 0x00007f27f34d84f0 Collection.map<A>(_:) + 463 in libswiftCore.so
12 URI.queryParameters.getter + 314 in <PROJ_NAME_REDACTED> at <PROJ_ROOT_REDACTED>/.build/checkouts/hummingbird/Sources/HummingbirdCore/Request/URI.swift:51:38
13 static APIv2.parseAPIKey(from:) + 107 in <PROJ_NAME_REDACTED> at <PROJ_ROOT_REDACTED>/Sources/webservice/APIv2/APIv2.swift:19:34
14 APIv2.Rates.respond(to:context:) + 1354 in <PROJ_NAME_REDACTED> at <PROJ_ROOT_REDACTED>/Sources/webservice/APIv2/RatesEndpoint.swift:20:17
15 RouterResponder.respond(to:context:) in <PROJ_NAME_REDACTED> at <PROJ_ROOT_REDACTED>/.build/checkouts/hummingbird/Sources/Hummingbird/Router/RouterResponder.swift:57
16 respond #1 <A>@Sendable (to:channel:) in ApplicationProtocol.run() in <PROJ_NAME_REDACTED> at <PROJ_ROOT_REDACTED>/.build/checkouts/hummingbird/Sources/Hummingbird/Application.swift:106
17 closure #1 in closure #1 in closure #1 in HTTPChannelHandler.handleHTTP(asyncChannel:logger:) in <PROJ_NAME_REDACTED> at <PROJ_ROOT_REDACTED>/.build/checkouts/hummingbird/Sources/HummingbirdCore/Server/HTTP/HTTPChannelHandler.swift:66
18 NIOAsyncChannel.executeThenClose<A>(_:) in <PROJ_NAME_REDACTED> at <PROJ_ROOT_REDACTED>/.build/checkouts/swift-nio/Sources/NIOCore/AsyncChannel/AsyncChannel.swift:271
19 closure #1 in closure #1 in HTTPChannelHandler.handleHTTP(asyncChannel:logger:) in <PROJ_NAME_REDACTED> at <PROJ_ROOT_REDACTED>/.build/checkouts/hummingbird/Sources/HummingbirdCore/Server/HTTP/HTTPChannelHandler.swift:48
20 withGracefulShutdownHandler<A>(operation:onGracefulShutdown:) in <PROJ_NAME_REDACTED> at <PROJ_ROOT_REDACTED>/.build/checkouts/swift-service-lifecycle/Sources/ServiceLifecycle/GracefulShutdown.swift:55
21 closure #1 in HTTPChannelHandler.handleHTTP(asyncChannel:logger:) in <PROJ_NAME_REDACTED> at <PROJ_ROOT_REDACTED>/.build/checkouts/hummingbird/Sources/HummingbirdCore/Server/HTTP/HTTPChannelHandler.swift:47
22 0x00007f27f333cce0 withTaskCancellationHandler<A>(operation:onCancel:) in libswift_Concurrency.so
23 HTTPChannelHandler.handleHTTP(asyncChannel:logger:) in <PROJ_NAME_REDACTED> at <PROJ_ROOT_REDACTED>/.build/checkouts/hummingbird/Sources/HummingbirdCore/Server/HTTP/HTTPChannelHandler.swift:46
24 HTTP1Channel.handle(value:logger:) in <PROJ_NAME_REDACTED> at <PROJ_ROOT_REDACTED>/.build/checkouts/hummingbird/Sources/HummingbirdCore/Server/HTTP/HTTP1Channel.swift:65
25 closure #1 in closure #1 in closure #1 in closure #1 in Server.run() in <PROJ_NAME_REDACTED> at <PROJ_ROOT_REDACTED>/.build/checkouts/hummingbird/Sources/HummingbirdCore/Server/Server.swift:119

Backtrace took 36.46s

Illegal instruction (core dumped)

Problem: Looks like I am getting some traffic with bad query params that are causing the HBParser code to overflow.

Solution: I simply guard against this potential overflow before it happens.

Result: My production service no longer reproduces the above stacktrace when I deploy using the commit proposed here.

[adam-fowler]

I'm wondering if you could implement this as two loops to reduce the number of checks eg

while index < original.endIndex - 2 {
...
}
while index < original.endIndex {
... ignore percent encoding
}

[adam-fowler]

This all looks good. Can we add a test to verify it.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.77%. Comparing base (e78cde7) to head (feb774c).
Report is 12 commits behind head on main.

❗ Current head feb774c differs from pull request most recent head ecbd7a8. Consider uploading reports for the commit ecbd7a8 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #404      +/-   ##
==========================================
- Coverage   84.86%   83.77%   -1.10%     
==========================================
  Files          98       99       +1     
  Lines        5320     4087    -1233     
==========================================
- Hits         4515     3424    -1091     
+ Misses        805      663     -142     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Joannis]

Thank you for finding, reporting and especially for fixing this!

[tannerdsilva]

oh snap I accidentally deleted a test case from the file while I was working on this :P

will force push the replacement here shortly....apologies