Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
# Unreleased

- ***(SECURITY)*** Prevent SSE injection in `Kemal::EventStream`: reject newlines in `event`/`id`, normalize CR/LF in `data`/`comment`. Thanks @hahwul for the report. Thanks @sdogruyol for the fix :pray:
- Fix URL params being decoded again on every request when route lookup results are cached. Thanks @hahwul for the report. Thanks @sdogruyol for the fix :pray:

# 1.12.0 (21-07-2026)

- Crystal 1.21.0 support :tada:
Expand Down
15 changes: 15 additions & 0 deletions spec/context_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -129,4 +129,19 @@ describe "Context" do
context.params.url["id"].should eq "42"
end
end

context "url params cache isolation" do
it "does not mutate cached radix params across repeated requests" do
get "/files/:path" do |env|
env.params.url["path"]
end

path = "/%252e%252e%252fsecret.txt"
first = call_request_on_app(HTTP::Request.new("GET", "/files#{path}"))
second = call_request_on_app(HTTP::Request.new("GET", "/files#{path}"))

first.body.should eq("%2e%2e%2fsecret.txt")
second.body.should eq("%2e%2e%2fsecret.txt")
end
end
end
51 changes: 51 additions & 0 deletions spec/event_stream_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,47 @@ describe Kemal::EventStream do
client_response.body.should eq("data: line one\ndata: line two\n\n")
end

it "rejects LF, CR, and CRLF in event names" do
{"tick\ndata: x", "tick\rdata: x", "tick\r\ndata: x"}.each do |event|
io = IO::Memory.new
stream = Kemal::EventStream.new(HTTP::Server::Response.new(io))
expect_raises(ArgumentError, "SSE event must not contain CR or LF") do
stream.send("hello", event: event)
end
end
end

it "rejects LF, CR, and CRLF in ids" do
{"7\nevent: x", "7\revent: x", "7\r\nevent: x"}.each do |id|
io = IO::Memory.new
stream = Kemal::EventStream.new(HTTP::Server::Response.new(io))
expect_raises(ArgumentError, "SSE id must not contain CR or LF") do
stream.send("hello", id: id)
end
end
end

it "treats bare CR in data as a line break to prevent field smuggling" do
sse "/events" do |stream, _|
stream.send("safe\rdata: smuggled-via-CR")
end

request = HTTP::Request.new("GET", "/events")
client_response = call_request_on_app(request)
client_response.body.should eq("data: safe\ndata: data: smuggled-via-CR\n\n")
client_response.body.should_not contain("\r")
end

it "treats CRLF in data as a line break to prevent field smuggling" do
sse "/events" do |stream, _|
stream.send("safe\r\nevent: injected\r\ndata: owned")
end

request = HTTP::Request.new("GET", "/events")
client_response = call_request_on_app(request)
client_response.body.should eq("data: safe\ndata: event: injected\ndata: data: owned\n\n")
end

it "sends keep-alive comments" do
sse "/events" do |stream, _|
stream.comment("ping")
Expand All @@ -45,6 +86,16 @@ describe Kemal::EventStream do
client_response.body.should eq(": ping\n\n")
end

it "splits multi-line comments to prevent SSE injection" do
sse "/events" do |stream, _|
stream.comment("ping\nevent: injected\ndata: owned")
end

request = HTTP::Request.new("GET", "/events")
client_response = call_request_on_app(request)
client_response.body.should eq(": ping\n: event: injected\n: data: owned\n\n")
end

it "supports url parameters" do
sse "/events/:channel" do |stream, env|
stream.send(env.params.url["channel"])
Expand Down
33 changes: 29 additions & 4 deletions src/kemal/event_stream.cr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ module Kemal
#
# Sets the required headers and formats events according to the SSE spec.
class EventStream
# SSE treats CR, LF, and CRLF as line terminators (WHATWG HTML).
private SSE_LINE_BREAK = /\r\n|\r|\n/

def initialize(@response : HTTP::Server::Response)
setup_headers
end
Expand All @@ -15,11 +18,20 @@ module Kemal
end

# Sends an SSE event. Multi-line *data* is split into separate `data:` fields.
# *event* and *id* must not contain CR/LF; newlines there raise `ArgumentError`
# so they cannot inject SSE fields.
def send(data : String, *, event : String? = nil, id : String | Int? = nil, retry : Time::Span? = nil) : self
@response.puts "event: #{event}" if event
@response.puts "id: #{id}" if id
if event
validate_single_line!("event", event)
@response.puts "event: #{event}"
end
if id
id_value = id.to_s
validate_single_line!("id", id_value)
@response.puts "id: #{id_value}"
end
@response.puts "retry: #{retry.total_milliseconds.to_i}" if retry
data.each_line(chomp: true) do |line|
each_sse_line(data) do |line|
@response.puts "data: #{line}"
end
@response.puts
Expand All @@ -29,7 +41,8 @@ module Kemal

# Sends a keep-alive comment (ignored by clients, useful during idle periods).
def comment(text : String) : self
@response.print ": #{text}\n\n"
each_sse_line(text) { |line| @response.print ": #{line}\n" }
@response.print "\n"
flush
self
end
Expand All @@ -42,6 +55,18 @@ module Kemal
@response.close
end

private def validate_single_line!(field : String, value : String) : Nil
if value.includes?('\n') || value.includes?('\r')
raise ArgumentError.new("SSE #{field} must not contain CR or LF")
end
end

# Yields each SSE line. Normalizes CR/CRLF only when needed to avoid an extra alloc.
private def each_sse_line(value : String, & : String ->) : Nil
value = value.gsub(SSE_LINE_BREAK, "\n") if value.includes?('\r')
value.each_line(chomp: true) { |line| yield line }
end

private def setup_headers
@response.content_type = "text/event-stream; charset=utf-8"
@response.headers["Cache-Control"] = "no-cache"
Expand Down
6 changes: 4 additions & 2 deletions src/kemal/param_parser.cr
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ module Kemal
alias AllParamTypes = String | Int64 | Float64 | Bool | Hash(String, JSON::Any) | Array(JSON::Any)?
getter files, all_files

def initialize(@request : HTTP::Request, @url : Hash(String, String) = {} of String => String)
def initialize(@request : HTTP::Request, url : Hash(String, String) = {} of String => String)
# Own a copy so in-place URI decode cannot mutate a shared/cached Radix params hash.
@url = url.dup
@query = HTTP::Params.new({} of String => Array(String))
@body = HTTP::Params.new({} of String => Array(String))
@json = {} of String => AllParamTypes
Expand Down Expand Up @@ -111,7 +113,7 @@ module Kemal

# Updates url params (e.g. after request method override). Used by Context#invalidate_route_cache.
def update_url_params(new_url : Hash(String, String))
@url = new_url
@url = new_url.dup
@url_parsed = false
end

Expand Down
Loading