Audit M5: cap the attachment download at its declared size
Header and read both bounded — contentLength is -1 on a chunked reply, so trusting it alone would leave the hole open. Same reasoning as iOS: the size came through the ratchet, so the relay cannot inflate it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
d1b0acdda6
commit
e435cd710d
@@ -345,7 +345,15 @@ class Conversations(
|
||||
|
||||
/** Download and open a blob a message referred to. */
|
||||
fun fetchAttachment(ref: Attachment.Ref): ByteArray =
|
||||
Attachment.open(relay.downloadAttachment(ref.id), ref)
|
||||
// The size came through the ratchet, so the relay cannot inflate it —
|
||||
// which is what makes it usable as a ceiling (audit M5).
|
||||
Attachment.open(
|
||||
relay.downloadAttachment(
|
||||
ref.id,
|
||||
maxBytes = if (ref.bytes > 0) minOf(ref.bytes + 4096L,
|
||||
RelayClient.HARD_ATTACHMENT_CAP)
|
||||
else RelayClient.HARD_ATTACHMENT_CAP),
|
||||
ref)
|
||||
|
||||
/**
|
||||
* Refuse to encrypt to keys we have reason to distrust.
|
||||
|
||||
@@ -49,6 +49,9 @@ class RelayClient(
|
||||
private val JSON = "application/json".toMediaType()
|
||||
private val OCTETS = "application/octet-stream".toMediaType()
|
||||
|
||||
/** The relay's own 10 MiB per-blob limit, with slack (audit M5). */
|
||||
const val HARD_ATTACHMENT_CAP = 12L * 1024 * 1024
|
||||
|
||||
fun defaultHttpClient(): OkHttpClient = OkHttpClient.Builder()
|
||||
.callTimeout(25, TimeUnit.SECONDS)
|
||||
// No response cache, deliberately. The payloads are ciphertext, but
|
||||
@@ -233,10 +236,41 @@ class RelayClient(
|
||||
request("POST", url("/e2e/attachments"), "/e2e/attachments", blob.toRequestBody(OCTETS))
|
||||
.use { json(it).getString("id") }
|
||||
|
||||
/** `GET /e2e/attachments/{id}`. The caller verifies the digest and decrypts. */
|
||||
fun downloadAttachment(id: String): ByteArray =
|
||||
/**
|
||||
* `GET /e2e/attachments/{id}`. The caller verifies the digest and decrypts.
|
||||
*
|
||||
* 🔴 **Bounded before it is read (audit M5).** `maxBytes` is the size the
|
||||
* sealed reference promised — it arrived through the ratchet, so the relay
|
||||
* cannot inflate it. Without a ceiling, `body.bytes()` reads whatever the
|
||||
* relay sends, and a malicious one answers a photo request with a
|
||||
* multi-gigabyte body that kills the app before the digest check it would
|
||||
* have failed.
|
||||
*/
|
||||
fun downloadAttachment(id: String, maxBytes: Long = HARD_ATTACHMENT_CAP): ByteArray =
|
||||
request("GET", url("/e2e/attachments/$id"), "/e2e/attachments/$id", null)
|
||||
.use { it.body?.bytes() ?: ByteArray(0) }
|
||||
.use { response ->
|
||||
val body = response.body ?: return@use ByteArray(0)
|
||||
if (body.contentLength() > maxBytes) {
|
||||
throw RelayException.Malformed("attachment is larger than it claimed to be")
|
||||
}
|
||||
// contentLength() is -1 on a chunked reply, so the READ is
|
||||
// capped too rather than trusting the header: take one byte
|
||||
// more than allowed and refuse if it arrives.
|
||||
val stream = body.byteStream()
|
||||
val out = java.io.ByteArrayOutputStream()
|
||||
val buf = ByteArray(64 * 1024)
|
||||
var total = 0L
|
||||
while (true) {
|
||||
val n = stream.read(buf)
|
||||
if (n < 0) break
|
||||
total += n
|
||||
if (total > maxBytes) {
|
||||
throw RelayException.Malformed("attachment exceeded its declared size")
|
||||
}
|
||||
out.write(buf, 0, n)
|
||||
}
|
||||
out.toByteArray()
|
||||
}
|
||||
|
||||
// -- Groups (§8.9) -------------------------------------------------------
|
||||
|
||||
|
||||
Reference in New Issue
Block a user