mirror of
https://github.com/immich-app/immich.git
synced 2026-05-18 03:10:24 +03:00
fix(mobile): cronet buffer overflow on compressed thumbnails (#28439)
CronetImageFetcher sized the response buffer from Content-Length, which is the compressed wire size. Cronet auto-decompresses gzip/br responses and writes decompressed bytes into the buffer, exceeding it and throwing IllegalArgumentException: ByteBuffer is already full on the next read. Use the growable path; Content-Length becomes an initial alloc hint only, capped at 128 MB so an untrusted server can't overflow Int.MAX_VALUE or OOM us upfront. Reuse Cronet's ByteBuffer between reads when no grow is needed.
This commit is contained in:
@@ -23,6 +23,8 @@ import java.io.IOException
|
|||||||
import java.nio.ByteBuffer
|
import java.nio.ByteBuffer
|
||||||
import java.util.concurrent.ConcurrentHashMap
|
import java.util.concurrent.ConcurrentHashMap
|
||||||
|
|
||||||
|
private const val MAX_PREALLOC_BYTES = 128 * 1024 * 1024
|
||||||
|
|
||||||
private class RemoteRequest(val cancellationSignal: CancellationSignal)
|
private class RemoteRequest(val cancellationSignal: CancellationSignal)
|
||||||
|
|
||||||
class RemoteImagesImpl(context: Context) : RemoteImageApi {
|
class RemoteImagesImpl(context: Context) : RemoteImageApi {
|
||||||
@@ -228,7 +230,6 @@ private class CronetImageFetcher : ImageFetcher {
|
|||||||
private val onComplete: () -> Unit,
|
private val onComplete: () -> Unit,
|
||||||
) : UrlRequest.Callback() {
|
) : UrlRequest.Callback() {
|
||||||
private var buffer: NativeByteBuffer? = null
|
private var buffer: NativeByteBuffer? = null
|
||||||
private var wrapped: ByteBuffer? = null
|
|
||||||
private var error: Exception? = null
|
private var error: Exception? = null
|
||||||
|
|
||||||
override fun onRedirectReceived(request: UrlRequest, info: UrlResponseInfo, newUrl: String) {
|
override fun onRedirectReceived(request: UrlRequest, info: UrlResponseInfo, newUrl: String) {
|
||||||
@@ -242,15 +243,16 @@ private class CronetImageFetcher : ImageFetcher {
|
|||||||
}
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
|
// Content-Length is a size hint only. With Content-Encoding (gzip/br/...),
|
||||||
|
// Cronet auto-decompresses and writes decompressed bytes to our buffer, which
|
||||||
|
// may exceed the wire/compressed Content-Length. Always use the growable
|
||||||
|
// buffer path so we can't overflow.
|
||||||
val contentLength = info.allHeaders["content-length"]?.firstOrNull()?.toIntOrNull() ?: 0
|
val contentLength = info.allHeaders["content-length"]?.firstOrNull()?.toIntOrNull() ?: 0
|
||||||
if (contentLength > 0) {
|
// Cap the up-front alloc: Content-Length is untrusted and can be huge or near
|
||||||
buffer = NativeByteBuffer(contentLength + 1)
|
// Int.MAX_VALUE (overflowing `+1`). For larger responses the grow path takes over.
|
||||||
wrapped = NativeBuffer.wrap(buffer!!.pointer, contentLength + 1)
|
val initialSize = if (contentLength in 1..MAX_PREALLOC_BYTES) contentLength + 1 else INITIAL_BUFFER_SIZE
|
||||||
request.read(wrapped)
|
buffer = NativeByteBuffer(initialSize)
|
||||||
} else {
|
request.read(buffer!!.wrapRemaining())
|
||||||
buffer = NativeByteBuffer(INITIAL_BUFFER_SIZE)
|
|
||||||
request.read(buffer!!.wrapRemaining())
|
|
||||||
}
|
|
||||||
} catch (e: Exception) {
|
} catch (e: Exception) {
|
||||||
error = e
|
error = e
|
||||||
return request.cancel()
|
return request.cancel()
|
||||||
@@ -263,14 +265,18 @@ private class CronetImageFetcher : ImageFetcher {
|
|||||||
byteBuffer: ByteBuffer
|
byteBuffer: ByteBuffer
|
||||||
) {
|
) {
|
||||||
try {
|
try {
|
||||||
val buf = if (wrapped == null) {
|
val b = buffer!!
|
||||||
buffer!!.run {
|
b.advance(byteBuffer.position())
|
||||||
advance(byteBuffer.position())
|
// Reuse the caller-supplied ByteBuffer as long as we don't need to grow.
|
||||||
ensureHeadroom()
|
// It already points at our native memory with position advanced past the
|
||||||
wrapRemaining()
|
// written bytes — Cronet can keep writing into the remaining tail.
|
||||||
}
|
// Only when the buffer is full do we grow (which may realloc + move the
|
||||||
|
// native pointer) and need a fresh wrap.
|
||||||
|
val buf = if (b.offset == b.capacity) {
|
||||||
|
b.ensureHeadroom()
|
||||||
|
b.wrapRemaining()
|
||||||
} else {
|
} else {
|
||||||
wrapped
|
byteBuffer
|
||||||
}
|
}
|
||||||
request.read(buf)
|
request.read(buf)
|
||||||
} catch (e: Exception) {
|
} catch (e: Exception) {
|
||||||
@@ -280,7 +286,6 @@ private class CronetImageFetcher : ImageFetcher {
|
|||||||
}
|
}
|
||||||
|
|
||||||
override fun onSucceeded(request: UrlRequest, info: UrlResponseInfo) {
|
override fun onSucceeded(request: UrlRequest, info: UrlResponseInfo) {
|
||||||
wrapped?.let { buffer!!.advance(it.position()) }
|
|
||||||
onSuccess(buffer!!)
|
onSuccess(buffer!!)
|
||||||
onComplete()
|
onComplete()
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user