SSE injection: bare \r and \n in scalar option values survive serialization

[andriytyurnikov]

Summary

Datastar::ServerSentEventGenerator writes scalar SSE option values (selector, mode, useViewTransition, attribute values, etc.) as single-line data: fields without sanitization. Bare \r is also preserved in element bodies and execute-script bodies because the generator splits inputs on \n only. The browser's SSE parser (WHATWG spec) treats \r, \n, and \r\n all as line terminators, so attacker-controlled content reaching any of these fields can forge SSE events that the client will dispatch.

Severity

Low–medium. Most call sites pass developer-authored constants for selectors and modes. Realistic exposure is when a CSS selector or attribute value is built from user input — e.g. selector: "#user-#{params[:id]}" where params[:id] could contain CR/LF. The fix is trivial; even if no production exploit exists today, the spec-correct serializer should never let opaque inputs break framing.

Affected version

datastar 1.0.4 (latest at time of writing). Verified.

Reproduction

require "datastar"
require "stringio"

io = StringIO.new
gen = Datastar::ServerSentEventGenerator.new(io, signals: {})

# Bare CR in element body
gen.patch_elements("<li>safe</li>\revent: forged\ndata: elements <pwned>")

# Bare CR in scalar option (selector)
gen.patch_elements("<p>x</p>", selector: "#a\rinjected: evil")

# Bare LF in scalar option
gen.patch_elements("<p>x</p>", selector: "#b\ninjected: evil")

# Bare CR in execute_script body
gen.execute_script("safe()\revent: forged\ndata: elements <pwned>")

# Browser splits on \r, \n, or \r\n
io.string.split(/\r\n|\r|\n/)
  .each_with_index { |l, i| puts "%2d: %s" % [i, l] }

Output (abridged) — 6 event: lines visible to the parser when only 4 method calls were made; rogue fields land between legitimate events:

 1: data: elements <li>safe</li>
 2: event: forged
 3: data: elements data: elements <pwned>
...
 6: data: selector #a
 7: injected: evil
...
11: data: selector #b
12: injected: evil
...
19: event: forged
20: data: elements data: elements <pwned></script>

Expected

Caller-supplied strings should never be able to forge or split SSE fields, regardless of where they appear in the API surface.

Suggested fix

Two scrubbers:

• Element/script bodies — strip \r only. (\n is intentional; the SDK already splits on it to emit per-line data: fields.)
• Scalar option values + array/hash option entries — strip both \r and \n. These are written as a single data: line by build_options, so any embedded line terminator forges a field.

Sketch:

# server_sent_event_generator.rb

CR = "\r"
CRLF = "\r\n"

def patch_elements(elements, options = BLANK_OPTIONS)
  elements = Array(elements).compact.map { |e| _scrub_body(e) }
  # ... existing logic with scrubbed elements and scrubbed options
end

private

def _scrub_body(value)
  return value unless value.is_a?(String)
  value.tr(CR, "")
end

def _scrub_option(value)
  case value
  when String then value.tr(CRLF, "")
  when Array  then value.map { |v| _scrub_option(v) }
  when Hash   then value.transform_values { |v| _scrub_option(v) }
  else value
  end
end

Apply _scrub_option to every value passing through build_options, and _scrub_body to elements/script bodies and String signal payloads. Ints/floats/bools are unaffected.

Workaround

Subclass ServerSentEventGenerator and override the public methods to scrub inputs before super. We're shipping this in vibes-datastar as class Stream < Datastar::ServerSentEventGenerator.

[andriytyurnikov]

Spiked a downstream migration onto Datastar::Dispatcher in our vibes-datastar wrapper and ran into one extra friction point that's adjacent to this issue and worth flagging.

We currently work around the \r / \n issue by subclassing ServerSentEventGenerator and overriding patch_elements / patch_signals / remove_elements / remove_signals / execute_script to scrub inputs before super. That works for our broadcast/datastar helpers (we instantiate the subclass ourselves), but breaks down inside Dispatcher#stream { |sse| ... }: the dispatcher hard-codes the generator class at Dispatcher#stream_one, so the sse user code receives is the un-scrubbed SDK class, not our subclass.

If the fix lands inside ServerSentEventGenerator itself, this becomes moot for us — great outcome.

If for some reason it doesn't, a smaller adjacent change that would also unblock us: let Dispatcher.new accept an optional generator_class: (defaulting to ServerSentEventGenerator), and use it at the two call sites in stream_one/stream_many. That's the standard "let the wrapper override the workhorse class" hook and would let any downstream gem layer behavior on top — scrubbing, logging, metrics, etc. — without monkey-patching.

Not asking for a PR yet, just naming the second order issue in case it influences the shape of the fix.

[andriytyurnikov]

Opened #22 with the canonical fix in ServerSentEventGenerator — both scrub_body (strip \r from element/script bodies, preserve \n for the splitter) and scrub_option (recursively strip \r/\n from option values, including arrays/hashes). 17 new specs cover element bodies, scalar/array/hash options, String and Hash signals, remove_elements, execute_script (body and attributes), redirect, and the original reproduction. Full suite: 117 examples, 0 failures.

Also opened #21 separately for the smaller adjacent change discussed above — generator_class: injection on Dispatcher.new — so downstream wrappers can layer behavior (scrubbing, logging, metrics) without monkey-patching, independent of when this lands.

Both PRs are independent and either can land first.