You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
(SECURITY) Never route a request-target that does not start with /, and refuse a malformed one (GHSA-x97g-6h7h-q9w6). The route key is the method followed by the path with nothing in between, so the request line GE T/admin/secret built the key of GET /admin/secret and was served by that route, while every path-scoped guard read T/admin/secret and stood aside: use "/admin", Auth, before_all "/admin/*", a Kemal::Router mounted at /admin, prefix only/exclude rules. Any verb worked - DELET E/admin/users/42 ran the delete route. Such a request is now answered 400 Bad Request with Connection: close, through a registered error 400 like the refusal of a malformed method (#820). The route tree, context.route and the only/exclude rules of a Kemal::Handler no longer resolve a method and a path that cannot both be believed, so a handler that answers by itself behind only ["/metrics"] no longer served GE T/metrics - nor GET/metrics /, which the method check of #820 left reaching it. Absolute-form (http://host/path), authority-form (CONNECT host:port) and OPTIONS * targets are well formed, routed nowhere and still get 404.
(SECURITY) Run Kemal::Router filters for every request the router's routes serve, ws routes included (GHSA-89xc-29wg-6mm5). A router's filters were registered by path only for its HTTP and SSE routes, so a router.before or router.before_get authentication guard let a WebSocket handshake to the same router through - an unauthenticated client reached the ws handler, also under namespace and mounted sub-routers. Registering them by path also let filters elsewhere decide whether they ran, and a filter path was compared with the spelling of the router's route paths, so a guard could silently not run for HTTP routes too. Router filters now travel with the router's routes, and a filter path is matched against the request path (router.before "/users/:id" covers a get "/users/me"). Thanks @barisern for the report 🙏
(BREAKING) Filters around a WebSocket handshake, following the fix above:
The GET filters run for a handshake - top-level and router, including a pathless before_get, which therefore runs on every handshake. A filter that assumes an HTTP route, such as env.route.path, raises there.
A filter refuses a handshake with halt, or with a status a registered error handler covers; the refusal closes the connection. A status alone still upgrades, as it still runs an HTTP route's handler.
after filters do not run for a handshake, since they would run once the 101 is on the wire - also where an HTTP route shares the path, which used to run them.
(BREAKING) A router's filters apply to the router's own routes only. A top-level route or ws on the same path no longer runs them; guard it with top-level filters. For a request no route serves, the router before filters of a route on the path still run, so a router guard or CORS preflight answer keeps covering the methods the router does not serve. Router filters run after the top-level filters of the same step.
(BREAKING) Run every top-level path filter whose path matches the request, in the order their paths were first registered, instead of only the most specific one. Global filters (before_all and friends without a path) still run first, router filters last. Before upgrading, check the filters whose paths overlap:
A more specific filter path no longer exempts its paths from a broader guard. With before_all "/api/*" { halt 401 unless authenticated } and any before_all "/api/login" filter, POST /api/login used to get past the guard and now answers 401. Skip such paths inside the guard itself, e.g. next if env.request.path == "/api/login".
Filters on overlapping paths all run, so side effects such as logging, counters or rate limits may run more than once per request: before_get "/admin" next to before_get "/admin/" both run for either spelling, since each matches both. One block registered for several paths at once, as with before_all ["/admin", "/admin/*"], still runs once.
The filter on the path registered later has the last word. before_all "/api/html" { |env| env.response.content_type = "text/html" } followed by before_all "/api/*" { |env| env.response.content_type = "application/json" } now answers /api/html with JSON; register the more specific path after the broader one. A block added to a path later joins that path's place, so registering /api/html again after /api/* does not help. after filters run in the same order, not reversed.
Why: path filters shared one radix tree that ran the single best match, so a broader guard silently did not run wherever a more specific filter path matched too: with before_all "/api/*" { halt 401 } and before_all "/api/public" { ... }, GET /api/public skipped the guard and got 200, and before_get "/users/:id" was skipped for GET /users/me once before_get "/users/me" existed. A filter path now matches what it matches on its own: the shared tree could also lose filters altogether, so before_get "/users/:id" and before_get "/users/:uid/posts" together ran for neither /users/5 nor /users/5/posts, and it refused some combinations at boot with Radix::Tree::SharedKeyError - add before_get "/users/me" to those two - which are now accepted. Kemal::FilterHandler#tree no longer holds the path filters.
(BREAKING) Run no before filter after one that closed the response with halt or redirect - not later blocks on the same path, path filters after a halting before_all, the before_get step after the before_all one, or router filters - just as it already kept the route handler from running. after filters still all run. Before upgrading:
A before filter that has to see refused requests - audit logging of failed logins, a metrics counter - no longer runs for them when it comes after the guard. Move it to an after filter.
Refuse with halt. A guard that only sets a status for a registered error handler leaves the response open, so the filters after it still run and can change the status.
An after filter that sets a header or the content type has to check env.response.closed? first: on a halted response it raises IO::Error, which skips the after filters behind it, after_all logging included.
Why: the filters after a halt kept running on a response already sent. With before_all "/api/*" { halt 401 } and before_get "/api/public" { |env| env.response.content_type = "application/json" }, the client got its 401 but content_type= raised IO::Error on the closed response, which skipped after_all, and a later filter's work - a database lookup, a counter - ran for a request the guard had refused. With overlapping path filters now all running, a halting guard with a filter behind it is the common case.
End the connection when an exception arrives after the response headers have gone out, instead of reading the next request on it. Kemal::ExceptionHandler could no longer render an error page at that point and returned as if the request had succeeded, so under keep-alive the server answered the next request on the same connection. A route that set Content-Length: 100, flushed partial and raised had the next pipelined response spliced into the 93 missing bytes - partialHTTP/1.1 200 OK... - which a client, or a proxy pooling the connection, read as one body. The connection now closes after the cut, as it does when an exception escapes the handlers altogether, so the client sees the response end short (curl: (18) end of response with 93 bytes missing). A chunked response, which the server still ends with its final chunk, and a response halt already closed keep their connection - an after filter touching a header of a large streamed response (#759) costs nothing. An error page for an exception raised before anything is sent is unchanged.
Leave a request path that names no static file to the routes instead of redirecting it. StaticFileHandler runs ahead of the routes and compared the decoded path with its canonical form, so an encoded / or a byte that is not UTF-8 in a route param never reached the route: with get "/u/:id", /u/a%2F%2Fb answered 302 /u/a/b and /u/%FF answered 302 /u/%EF%BF%BD. A non-canonical path is now redirected only when it names a file under public_folder, or when it has a . or .. segment - so /u/..%2F..%2Fetc still answers 302 /etc and no route receives a dot segment the handler would have resolved away.
Fix content_for in a partial rendered with render "partial.ecr" inside render(view, layout). The partial was rendered into a String::Builder, and its block kept writing into that builder, which had already been turned into the partial's string by the time the layout's yield_content ran the block. So <% content_for "meta" do %><title>From partial</title><% end %> in a partial left <%= yield_content "meta" %> empty, and the late write grew the finished builder, freeing the memory behind the string the partial had returned: a view that kept that string and printed it after yield_content sent whatever the heap held there - route keys, CLI option names - instead of the partial. A content_for block now receives the buffer yield_content collects it in as an argument, so the block reaches the layout from a partial as it does from the view, and a partial is rendered into an IO::Memory, so its string can no longer be overwritten.
Honour If-Range in send_file and the static file handler, per RFC 9110 §13.1.5. A Range was served whenever it parsed, so Range: bytes=0-3 with If-Range: "stale-etag" or If-Range: Mon, 01 Jan 2001 00:00:00 GMT got 206 and the first bytes of the current file, which a resuming client appended to the prefix it had of an older one, corrupting the download silently. A Range is now only honoured when If-Range names the response's validator: an entity tag by strong comparison (so Kemal's weak W/"..." tags never match), a date only when it equals the response's Last-Modified. Otherwise the full file is sent with 200. A route calling send_file without setting Last-Modified or ETag has nothing to compare, so a request carrying If-Range gets the full file there.
Close the connection after every refused WebSocket handshake, not only Kemal's own 403 and 405. Once the Origin and method checks passed, a wrong Sec-WebSocket-Version (426) or a missing Sec-WebSocket-Key (400) was answered by Crystal's HTTP::WebSocketHandler without Connection: close, so with Connection: keep-alive, Upgrade a request pipelined behind the handshake was served: GET /chat with Sec-WebSocket-Version: 1337 followed by GET /internal got 426 and then 200 OK with the internal response. Kemal now refuses both itself with Connection: close (the 426 still carries Sec-WebSocket-Version: 13), after the Origin check, and the pipelined request is never read. Completes the fix for GHSA-gvhf-4v24-f8r5.