mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-08 23:54:39 +00:00
fix(quic): bound peer-controlled buffers and channels — DoS hardening
Round 2 of the audit follow-ups. Each item caps a peer-controlled allocation that pre-fix could be inflated to hold gigabytes of heap or pin a CPU core indefinitely. * Http3FrameReader: cap pending unparsed buffer (1 MiB) and per-frame body length (16 MiB). A peer streaming a partial-frame prefix without ever delivering the body now raises QuicCodecException instead of growing buf indefinitely. * CapsuleReader: cap pending buffer (1 MiB) and per-capsule body (64 KiB). Symmetric encoder-side check on WT_CLOSE_SESSION reason size, matching the existing decoder cap. * WtPeerStreamDemux: replace Channel.UNLIMITED with bounded channels + suspending sends. readyStreams now caps queued peer-initiated streams at 1024; per-stream chunkChannel caps at 64 chunks. The collector's suspending send naturally back-pressures via QUIC flow control when the application is slow, rather than pinning heap. * WtDatagram.decode: validate quarter-id is in [0, (2^62-1)/4] so `r.value * 4` cannot overflow Long and wrap into a small signed value matching our session id (cross-session datagram injection). * QuicReader.readBytes / skip: translate negative-count into typed QuicCodecException instead of letting IllegalArgumentException escape from copyOfRange. * AckTracker: cap stored disjoint ranges at 64. A peer that sends alternating-bit-pattern PNs can no longer grow our ACK frame past what fits in a packet; oldest range evicts on overflow. * JcaAesGcmAead: track recent encrypt nonces (8) instead of just the most-recent, so a single intermediate seal between two rebuilds can't mask a duplicate against the second-most-recent. Drop the remembered nonce on doFinal failure so a retry takes the safe fresh-Cipher path. Add synchronized() defence-in-depth. Each cap has a generous default (above any legitimate use) but finite. Tests use no-arg construction; existing call sites unaffected. All 269 :quic:jvmTest tests pass. https://claude.ai/code/session_01AhGvbMV8uPRse3TmAGaddM
This commit is contained in:
@@ -203,6 +203,9 @@ class QuicReader(
|
||||
}
|
||||
|
||||
fun skip(n: Int) {
|
||||
if (n < 0) {
|
||||
throw QuicCodecException("skip with negative count: $n")
|
||||
}
|
||||
require(n)
|
||||
pos += n
|
||||
}
|
||||
@@ -248,6 +251,14 @@ class QuicReader(
|
||||
}
|
||||
|
||||
fun readBytes(n: Int): ByteArray {
|
||||
// Translate a negative `n` (e.g. an attacker-controlled length
|
||||
// that wrapped through `.toInt()`) into a typed
|
||||
// [QuicCodecException] instead of `IllegalArgumentException`
|
||||
// from `copyOfRange`. Bounds-checking via [require] still kicks
|
||||
// in for positive `n` past `end`.
|
||||
if (n < 0) {
|
||||
throw QuicCodecException("readBytes with negative count: $n")
|
||||
}
|
||||
require(n)
|
||||
val out = src.copyOfRange(pos, pos + n)
|
||||
pos += n
|
||||
|
||||
@@ -36,11 +36,27 @@ import com.vitorpamplona.quic.Varint
|
||||
* Unknown frame types (per RFC 9114 §9 — "frame types in the format and intent
|
||||
* not recognized SHOULD be ignored") are surfaced as [Http3Frame.Unknown] so
|
||||
* the caller can decide policy. SETTINGS, HEADERS, DATA are typed.
|
||||
*
|
||||
* Memory safety:
|
||||
* * Pending buffered bytes are capped at [maxPendingBytes]; a peer that
|
||||
* streams a partial-frame prefix without ever delivering the body
|
||||
* cannot pin unbounded heap. Exceeding the cap throws
|
||||
* [QuicCodecException], which the caller surfaces as a connection
|
||||
* error.
|
||||
* * Per-frame body length is capped by [maxFrameBodyBytes]. A peer that
|
||||
* advertises a 2 GiB length on the wire is rejected before the
|
||||
* `ByteArray(len.toInt())` allocation.
|
||||
*/
|
||||
class Http3FrameReader {
|
||||
class Http3FrameReader(
|
||||
private val maxPendingBytes: Int = DEFAULT_MAX_PENDING_BYTES,
|
||||
private val maxFrameBodyBytes: Int = DEFAULT_MAX_FRAME_BODY_BYTES,
|
||||
) {
|
||||
private var buf: ByteArray = ByteArray(0)
|
||||
private var pos: Int = 0
|
||||
|
||||
/** Bytes currently buffered awaiting a complete frame. Diagnostic; tests use this. */
|
||||
val bufferedBytes: Int get() = buf.size - pos
|
||||
|
||||
fun push(bytes: ByteArray) {
|
||||
if (bytes.isEmpty()) return
|
||||
// Amortized compaction: only shift bytes down when the consumed
|
||||
@@ -50,6 +66,14 @@ class Http3FrameReader {
|
||||
buf = buf.copyOfRange(pos, buf.size)
|
||||
pos = 0
|
||||
}
|
||||
val newPending = (buf.size - pos).toLong() + bytes.size.toLong()
|
||||
if (newPending > maxPendingBytes) {
|
||||
throw QuicCodecException(
|
||||
"HTTP/3 frame reader buffer would exceed cap " +
|
||||
"($newPending > $maxPendingBytes); peer is streaming a partial " +
|
||||
"frame without delivering the body — likely H3_EXCESSIVE_LOAD",
|
||||
)
|
||||
}
|
||||
val combined = ByteArray(buf.size + bytes.size)
|
||||
buf.copyInto(combined, 0)
|
||||
bytes.copyInto(combined, buf.size)
|
||||
@@ -63,8 +87,10 @@ class Http3FrameReader {
|
||||
val lenRes = Varint.decode(buf, typeEnd) ?: return null
|
||||
val bodyStart = typeEnd + lenRes.bytesConsumed
|
||||
val len = lenRes.value
|
||||
if (len < 0 || len > Int.MAX_VALUE.toLong()) {
|
||||
throw QuicCodecException("HTTP/3 frame length out of range: $len")
|
||||
if (len < 0 || len > maxFrameBodyBytes.toLong()) {
|
||||
throw QuicCodecException(
|
||||
"HTTP/3 frame length out of range: $len (cap $maxFrameBodyBytes)",
|
||||
)
|
||||
}
|
||||
val bodyEnd = bodyStart + len.toInt()
|
||||
if (bodyEnd > buf.size) return null // not all body bytes present yet
|
||||
@@ -78,6 +104,28 @@ class Http3FrameReader {
|
||||
else -> Http3Frame.Unknown(typeRes.value, body)
|
||||
}
|
||||
}
|
||||
|
||||
companion object {
|
||||
/**
|
||||
* Default cap on pending unparsed bytes (1 MiB). This is the
|
||||
* "frame in flight but body not yet complete" headroom — more
|
||||
* than enough for a legitimate HEADERS / SETTINGS / WT capsule
|
||||
* preamble plus a single in-progress DATA chunk, far less than
|
||||
* the heap a hostile peer could pin.
|
||||
*/
|
||||
const val DEFAULT_MAX_PENDING_BYTES: Int = 1 shl 20
|
||||
|
||||
/**
|
||||
* Default cap on a single HTTP/3 frame body (16 MiB). Exceeds
|
||||
* any plausible HEADERS payload (RFC 9114 only ever expects
|
||||
* tens of KiB) and any legitimate per-frame DATA chunk that
|
||||
* fits inside QUIC's per-packet payload budget. Setting this
|
||||
* lower than [DEFAULT_MAX_PENDING_BYTES] would require
|
||||
* splitting; setting it equal keeps the two caps aligned for
|
||||
* single-frame stalls.
|
||||
*/
|
||||
const val DEFAULT_MAX_FRAME_BODY_BYTES: Int = 16 shl 20
|
||||
}
|
||||
}
|
||||
|
||||
/** A parsed HTTP/3 frame. */
|
||||
|
||||
@@ -55,6 +55,7 @@ class AckTracker {
|
||||
val r = ranges[i]
|
||||
if (packetNumber > r.endInclusive + 1) {
|
||||
ranges.add(i, LongRange(packetNumber, packetNumber))
|
||||
trimToMaxRanges()
|
||||
return
|
||||
}
|
||||
if (packetNumber == r.endInclusive + 1) {
|
||||
@@ -80,6 +81,25 @@ class AckTracker {
|
||||
}
|
||||
}
|
||||
ranges += LongRange(packetNumber, packetNumber)
|
||||
trimToMaxRanges()
|
||||
}
|
||||
|
||||
/**
|
||||
* Cap [ranges] at [MAX_STORED_RANGES] by dropping the oldest entries
|
||||
* (the smallest PNs, at the tail of the descending list). Without
|
||||
* this a peer that sends a continuous stream of sparse PNs (every
|
||||
* other one, alternating-bit pattern) inflates the range list
|
||||
* unboundedly until the encoded ACK frame no longer fits in a
|
||||
* packet. The trade-off is that the very oldest ranges become
|
||||
* implicitly "unknown to us"; if the peer retransmits a packet from
|
||||
* an evicted range we'll re-deliver it and the deduplication is
|
||||
* downstream's problem (per-stream offset bookkeeping). Spurious
|
||||
* retransmits on truly ancient PNs are negligible in practice.
|
||||
*/
|
||||
private fun trimToMaxRanges() {
|
||||
while (ranges.size > MAX_STORED_RANGES) {
|
||||
ranges.removeAt(ranges.lastIndex)
|
||||
}
|
||||
}
|
||||
|
||||
fun hasUnackedAckEliciting(): Boolean = ackElicitingPending
|
||||
@@ -160,4 +180,14 @@ class AckTracker {
|
||||
additionalRanges = rest,
|
||||
)
|
||||
}
|
||||
|
||||
companion object {
|
||||
/**
|
||||
* Cap on stored disjoint ranges. 64 is far above any healthy-link
|
||||
* count (a single in-order receive needs 1 range; tens of
|
||||
* reorders per second push that to 4–8). Above this we evict
|
||||
* oldest first.
|
||||
*/
|
||||
const val MAX_STORED_RANGES: Int = 64
|
||||
}
|
||||
}
|
||||
|
||||
@@ -48,12 +48,22 @@ fun encodeCloseSessionCapsule(
|
||||
errorCode: Int = 0,
|
||||
reason: String = "",
|
||||
): ByteArray {
|
||||
val reasonBytes = reason.encodeToByteArray()
|
||||
// Symmetric with the decoder: draft-ietf-webtrans-http3 §5 caps the
|
||||
// reason string at 8192 bytes. Catching this on encode means a local
|
||||
// bug doesn't ship a capsule the peer must reject.
|
||||
require(reasonBytes.size <= MAX_WT_CLOSE_REASON_BYTES) {
|
||||
"WT_CLOSE_SESSION reason exceeds $MAX_WT_CLOSE_REASON_BYTES bytes (${reasonBytes.size})"
|
||||
}
|
||||
val body = QuicWriter()
|
||||
body.writeUint32(errorCode)
|
||||
body.writeBytes(reason.encodeToByteArray())
|
||||
body.writeBytes(reasonBytes)
|
||||
return encodeCapsule(WtCapsuleType.WT_CLOSE_SESSION, body.toByteArray())
|
||||
}
|
||||
|
||||
/** draft-ietf-webtrans-http3 §5 cap on the reason string in WT_CLOSE_SESSION. */
|
||||
const val MAX_WT_CLOSE_REASON_BYTES: Int = 8192
|
||||
|
||||
/** Parsed WT_CLOSE_SESSION capsule. */
|
||||
data class WtCloseSession(
|
||||
val errorCode: Int,
|
||||
@@ -64,11 +74,22 @@ data class WtCloseSession(
|
||||
* Stateful capsule reader. Capsules on the WT CONNECT bidi stream are
|
||||
* `(varint type)(varint length)(body)`. Feed bytes via [push]; drain
|
||||
* complete capsules via [next].
|
||||
*
|
||||
* Memory safety: pending unparsed bytes are capped at [maxPendingBytes]
|
||||
* and per-capsule body size is capped at [maxCapsuleBodyBytes]. A peer
|
||||
* that streams a partial capsule prefix (or advertises a 4 GiB length)
|
||||
* cannot pin unbounded heap.
|
||||
*/
|
||||
class CapsuleReader {
|
||||
class CapsuleReader(
|
||||
private val maxPendingBytes: Int = DEFAULT_MAX_PENDING_BYTES,
|
||||
private val maxCapsuleBodyBytes: Int = DEFAULT_MAX_CAPSULE_BODY_BYTES,
|
||||
) {
|
||||
private var buf: ByteArray = ByteArray(0)
|
||||
private var pos: Int = 0
|
||||
|
||||
/** Bytes currently buffered awaiting a complete capsule. Diagnostic. */
|
||||
val bufferedBytes: Int get() = buf.size - pos
|
||||
|
||||
fun push(bytes: ByteArray) {
|
||||
if (bytes.isEmpty()) return
|
||||
// Amortized compaction (same pattern as Http3FrameReader).
|
||||
@@ -76,6 +97,14 @@ class CapsuleReader {
|
||||
buf = buf.copyOfRange(pos, buf.size)
|
||||
pos = 0
|
||||
}
|
||||
val newPending = (buf.size - pos).toLong() + bytes.size.toLong()
|
||||
if (newPending > maxPendingBytes) {
|
||||
throw com.vitorpamplona.quic.QuicCodecException(
|
||||
"WT capsule reader buffer would exceed cap " +
|
||||
"($newPending > $maxPendingBytes); peer is streaming a partial " +
|
||||
"capsule without delivering the body",
|
||||
)
|
||||
}
|
||||
val combined = ByteArray(buf.size + bytes.size)
|
||||
buf.copyInto(combined, 0)
|
||||
bytes.copyInto(combined, buf.size)
|
||||
@@ -97,8 +126,10 @@ class CapsuleReader {
|
||||
.decode(buf, typeEnd) ?: return null
|
||||
val bodyStart = typeEnd + lenRes.bytesConsumed
|
||||
val len = lenRes.value
|
||||
if (len < 0 || len > Int.MAX_VALUE.toLong()) {
|
||||
throw com.vitorpamplona.quic.QuicCodecException("capsule length out of range: $len")
|
||||
if (len < 0 || len > maxCapsuleBodyBytes.toLong()) {
|
||||
throw com.vitorpamplona.quic.QuicCodecException(
|
||||
"capsule length out of range: $len (cap $maxCapsuleBodyBytes)",
|
||||
)
|
||||
}
|
||||
val bodyEnd = bodyStart + len.toInt()
|
||||
if (bodyEnd > buf.size) return null
|
||||
@@ -137,4 +168,16 @@ class CapsuleReader {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
companion object {
|
||||
/** Per-stream pending-capsule cap (1 MiB). */
|
||||
const val DEFAULT_MAX_PENDING_BYTES: Int = 1 shl 20
|
||||
|
||||
/**
|
||||
* Per-capsule body cap (64 KiB). The largest legitimate capsule
|
||||
* we expect is WT_CLOSE_SESSION (≤ [MAX_WT_CLOSE_REASON_BYTES] +
|
||||
* 4 bytes); 64 KiB is generous headroom for future capsule types.
|
||||
*/
|
||||
const val DEFAULT_MAX_CAPSULE_BODY_BYTES: Int = 64 * 1024
|
||||
}
|
||||
}
|
||||
|
||||
@@ -45,15 +45,28 @@ object WtDatagram {
|
||||
return w.toByteArray()
|
||||
}
|
||||
|
||||
/** Returns (quarterStreamId * 4, payload). Returns null on truncation. */
|
||||
/** Returns (quarterStreamId * 4, payload). Returns null on truncation or out-of-range quarter id. */
|
||||
fun decode(bytes: ByteArray): Decoded? {
|
||||
val r = Varint.decode(bytes, 0) ?: return null
|
||||
if (r.bytesConsumed > bytes.size) return null
|
||||
// QUIC stream ids are bounded by 2^62 - 1 (RFC 9000 §16). The
|
||||
// quarter id is the actual stream id divided by 4, so a valid
|
||||
// wire value is at most (2^62 - 1) / 4. Anything larger is
|
||||
// either malformed or a deliberate attempt to overflow `r.value
|
||||
// * 4` past `Long.MAX_VALUE` and have it wrap into a small
|
||||
// signed value that matches our expected session id — which
|
||||
// would let an off-path peer feed forged datagrams to the
|
||||
// application bypassing the session-id check in
|
||||
// [QuicWebTransportSessionState.pollIncomingDatagram].
|
||||
if (r.value < 0 || r.value > MAX_VALID_QUARTER_ID) return null
|
||||
val sessionId = r.value * 4
|
||||
val payload = bytes.copyOfRange(r.bytesConsumed, bytes.size)
|
||||
return Decoded(sessionId, payload)
|
||||
}
|
||||
|
||||
/** (2^62 - 1) / 4 — see [decode]. */
|
||||
internal const val MAX_VALID_QUARTER_ID: Long = ((1L shl 62) - 1L) / 4L
|
||||
|
||||
data class Decoded(
|
||||
val sessionStreamId: Long,
|
||||
val payload: ByteArray,
|
||||
|
||||
+40
-4
@@ -82,8 +82,20 @@ class WtPeerStreamDemux(
|
||||
* can leave this null.
|
||||
*/
|
||||
private val driver: com.vitorpamplona.quic.connection.QuicConnectionDriver? = null,
|
||||
/**
|
||||
* Cap on the number of peer-initiated streams that may queue here
|
||||
* waiting for the application to consume them. Pre-fix this was
|
||||
* `Channel.UNLIMITED` — a peer that opens streams faster than the
|
||||
* application drains could pin one [StrippedWtStream] (and its
|
||||
* captured `chunkChannel`) per stream id, indefinitely. Bounded
|
||||
* buffer + suspending `send` propagates backpressure: when the app
|
||||
* is slow, [route] suspends inside [emitStripped] before launching
|
||||
* any further per-stream work, which in turn lets QUIC's per-conn
|
||||
* `MAX_STREAMS_*` accounting throttle the peer.
|
||||
*/
|
||||
private val readyStreamsBuffer: Int = DEFAULT_READY_STREAMS_BUFFER,
|
||||
) {
|
||||
private val readyStreams = Channel<StrippedWtStream>(Channel.UNLIMITED)
|
||||
private val readyStreams = Channel<StrippedWtStream>(readyStreamsBuffer)
|
||||
|
||||
@Volatile
|
||||
var peerSettings: Http3Settings? = null
|
||||
@@ -134,7 +146,16 @@ class WtPeerStreamDemux(
|
||||
kotlinx.coroutines.coroutineScope {
|
||||
val pending = ArrayDeque<ByteArray>()
|
||||
val flowIterator = stream.incoming
|
||||
val chunkChannel = Channel<ByteArray>(Channel.UNLIMITED)
|
||||
// Bounded suspending channel — pre-fix `UNLIMITED` let a
|
||||
// single misbehaving peer stream pin gigabytes of heap when
|
||||
// the consumer (the application) couldn't keep up. With a
|
||||
// bounded buffer + suspending `send`, the collector below
|
||||
// back-pressures the QuicStream's incoming flow — which in
|
||||
// turn delays our outbound MAX_STREAM_DATA grants and
|
||||
// throttles the peer at the QUIC layer. The fixed size is
|
||||
// small (64 chunks ≈ ~64 KiB at typical packet payloads):
|
||||
// anything bigger just delays the back-pressure signal.
|
||||
val chunkChannel = Channel<ByteArray>(CHUNK_CHANNEL_BUFFER)
|
||||
val collector =
|
||||
launch {
|
||||
try {
|
||||
@@ -289,7 +310,7 @@ class WtPeerStreamDemux(
|
||||
}
|
||||
}
|
||||
|
||||
private fun emitStripped(
|
||||
private suspend fun emitStripped(
|
||||
stream: QuicStream,
|
||||
pending: ArrayDeque<ByteArray>,
|
||||
chunkChannel: Channel<ByteArray>,
|
||||
@@ -324,7 +345,14 @@ class WtPeerStreamDemux(
|
||||
driver?.wakeup()
|
||||
}
|
||||
}
|
||||
readyStreams.trySend(
|
||||
// Suspending send instead of `trySend` so a slow application
|
||||
// back-pressures the demux instead of silently dropping the
|
||||
// stream. With a bounded buffer ([readyStreamsBuffer]),
|
||||
// [readyStreams.send] suspends when the app hasn't drained,
|
||||
// which keeps `route()` busy and (combined with [process]'s
|
||||
// `scope.launch`) lets the next peer stream wait its turn
|
||||
// rather than spawning a coroutine for every offered id.
|
||||
readyStreams.send(
|
||||
StrippedWtStream(
|
||||
streamId = stream.streamId,
|
||||
isUnidirectional = isUni,
|
||||
@@ -334,6 +362,14 @@ class WtPeerStreamDemux(
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
companion object {
|
||||
/** Default cap on queued peer-initiated streams (1024). */
|
||||
const val DEFAULT_READY_STREAMS_BUFFER: Int = 1024
|
||||
|
||||
/** Per-stream chunk-channel capacity (64). */
|
||||
internal const val CHUNK_CHANNEL_BUFFER: Int = 64
|
||||
}
|
||||
}
|
||||
|
||||
private fun flatten(chunks: ArrayDeque<ByteArray>): ByteArray {
|
||||
|
||||
Reference in New Issue
Block a user