diff --git a/services/flipcash/src/main/kotlin/com/flipcash/services/controllers/BlobStorageController.kt b/services/flipcash/src/main/kotlin/com/flipcash/services/controllers/BlobStorageController.kt index bdd3675ec9..c79da4343f 100644 --- a/services/flipcash/src/main/kotlin/com/flipcash/services/controllers/BlobStorageController.kt +++ b/services/flipcash/src/main/kotlin/com/flipcash/services/controllers/BlobStorageController.kt @@ -1,6 +1,7 @@ package com.flipcash.services.controllers import com.flipcash.services.BlobUploader +import com.flipcash.services.internal.extensions.withoutJpegMetadata import com.flipcash.services.models.BlobNotReadyException import com.flipcash.services.models.BlobRejectedException import com.flipcash.services.models.blob.UploadPolicy @@ -49,6 +50,12 @@ class BlobStorageController @Inject constructor( suspend fun upload(bytes: ByteArray, mimeType: String): Result { val owner = owner() ?: return noAccount() + // Strip EXIF/GPS/XMP here, at the choke point every upload passes through, rather than + // trusting each call site to have already re-encoded its bytes clean. The reservation + // signs the byte count, so this must run before it — the size reserved has to match the + // size actually uploaded. + val bytes = bytes.withoutJpegMetadata() + val reservation = repository.initiateExternalUpload(mimeType, bytes.size.toLong(), owner) .getOrElse { return Result.failure(it) } diff --git a/services/flipcash/src/main/kotlin/com/flipcash/services/internal/extensions/JpegMetadata.kt b/services/flipcash/src/main/kotlin/com/flipcash/services/internal/extensions/JpegMetadata.kt new file mode 100644 index 0000000000..a2e331b4a4 --- /dev/null +++ b/services/flipcash/src/main/kotlin/com/flipcash/services/internal/extensions/JpegMetadata.kt @@ -0,0 +1,93 @@ +package com.flipcash.services.internal.extensions + +/** + * The APPn segments a stripped JPEG may keep: JFIF, the ICC colour profile, and Adobe's colour + * transform. None carries personal data, and dropping the colour ones visibly shifts a wide-gamut + * photo. + */ +private val ALLOWED_APP_MARKERS = setOf(0xE0, 0xE2, 0xEE) + +private const val MARKER_SOI = 0xD8 +private const val MARKER_TEM = 0x01 +private const val MARKER_SOS = 0xDA +private const val MARKER_EOI = 0xD9 +private const val MARKER_COM = 0xFE + +/** + * This JPEG without the segments that can carry personal data — EXIF (GPS, device serials), XMP, + * every other APPn outside the allowlist, and free-form comments. + * + * Returns the receiver untouched when it carries none, and when it doesn't parse: the decoder + * rejects a malformed stream on its own, and a half-rewritten one would be worse. + * + * Rotation must already be baked into the pixels, since stripping EXIF discards the orientation tag + * that decoders rely on. + * + * Must agree byte-for-byte with iOS's `JPEGMetadata.stripped` + * (`FlipcashCore/Sources/FlipcashCore/Blob/JPEGMetadata.swift`). + */ +internal fun ByteArray.withoutJpegMetadata(): ByteArray { + val segments = privacySegments() + if (segments.isNullOrEmpty()) return this + + val output = ByteArray(size - segments.sumOf { it.last - it.first + 1 }) + var copiedUpTo = 0 + var writeAt = 0 + for (segment in segments) { + copyInto(output, writeAt, copiedUpTo, segment.first) + writeAt += segment.first - copiedUpTo + copiedUpTo = segment.last + 1 + } + copyInto(output, writeAt, copiedUpTo, size) + + return output +} + +/** + * Walks the marker segments and returns the byte ranges to drop, or null when the stream doesn't + * parse. Stops at the scan — everything past it is entropy-coded pixel data. + */ +private fun ByteArray.privacySegments(): List? { + if (size < 2 || byteAt(0) != 0xFF || byteAt(1) != MARKER_SOI) return null + + val segments = mutableListOf() + var position = 2 + + while (position + 1 < size) { + if (byteAt(position) != 0xFF) return null + + val marker = byteAt(position + 1) + + // A marker may be padded with any number of 0xFF fill bytes. + if (marker == 0xFF) { + position += 1 + continue + } + + // Standalone markers, the RSTn restart markers included, carry no payload. + if (marker == MARKER_SOI || marker == MARKER_TEM || marker in 0xD0..0xD7) { + position += 2 + continue + } + + if (marker == MARKER_SOS || marker == MARKER_EOI) return segments + + if (position + 4 > size) return null + + val length = (byteAt(position + 2) shl 8) or byteAt(position + 3) + if (length < 2 || position + 2 + length > size) return null + + val carriesPersonalData = marker == MARKER_COM || + (marker in 0xE0..0xEF && marker !in ALLOWED_APP_MARKERS) + + if (carriesPersonalData) { + segments.add(position until position + 2 + length) + } + + position += 2 + length + } + + return segments +} + +private fun ByteArray.byteAt(index: Int): Int = this[index].toInt() and 0xFF diff --git a/services/flipcash/src/test/kotlin/com/flipcash/services/controllers/BlobStorageControllerTest.kt b/services/flipcash/src/test/kotlin/com/flipcash/services/controllers/BlobStorageControllerTest.kt index 5c6fc3f3c9..f8e21309fb 100644 --- a/services/flipcash/src/test/kotlin/com/flipcash/services/controllers/BlobStorageControllerTest.kt +++ b/services/flipcash/src/test/kotlin/com/flipcash/services/controllers/BlobStorageControllerTest.kt @@ -1,6 +1,7 @@ package com.flipcash.services.controllers import com.flipcash.services.BlobUploader +import com.flipcash.services.internal.extensions.withoutJpegMetadata import com.flipcash.services.models.BlobNotReadyException import com.flipcash.services.models.BlobRejectedException import com.flipcash.services.models.blob.UploadReservation @@ -19,9 +20,11 @@ import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every import io.mockk.mockk +import io.mockk.slot import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runTest import kotlin.test.Test +import kotlin.test.assertContentEquals import kotlin.test.assertEquals import kotlin.test.assertIs import kotlin.test.assertTrue @@ -168,6 +171,31 @@ class BlobStorageControllerTest { assertEquals(blobId, result.getOrNull()) } + @Test + fun `upload strips EXIF metadata and reserves the stripped size`() = runTest { + stubOwner() + val jpegWithExif = byteArrayOf(0xFF.toByte(), 0xD8.toByte()) + + byteArrayOf(0xFF.toByte(), 0xE1.toByte(), 0x00, 0x11) + + "Exif\u0000\u0000GPS 51.5N".toByteArray() + + byteArrayOf(0xFF.toByte(), 0xDA.toByte(), 0x00, 0x02, 0xFF.toByte(), 0xD9.toByte()) + val stripped = jpegWithExif.withoutJpegMetadata() + check(stripped.size < jpegWithExif.size) { "fixture must actually carry a stripped segment" } + + val sizes = mutableListOf() + coEvery { repository.initiateExternalUpload(any(), capture(sizes), any()) } returns + Result.success(UploadReservation(blobId, target)) + val uploadedBytes = slot() + coEvery { uploader.upload(capture(uploadedBytes), any(), any()) } returns Result.success(Unit) + coEvery { repository.completeExternalUpload(any(), any()) } returns Result.success(BlobStatus.PROCESSING) + coEvery { repository.getBlobs(any(), any(), any()) } returns Result.success(listOf(readyBlob())) + + val result = controller.upload(jpegWithExif, "image/jpeg") + + assertEquals(blobId, result.getOrNull()) + assertContentEquals(stripped, uploadedBytes.captured) + assertEquals(stripped.size.toLong(), sizes.single()) + } + @Test fun `getUploadPolicy fails without an account`() = runTest { every { userManager.accountCluster } returns null diff --git a/services/flipcash/src/test/kotlin/com/flipcash/services/internal/extensions/JpegMetadataTest.kt b/services/flipcash/src/test/kotlin/com/flipcash/services/internal/extensions/JpegMetadataTest.kt new file mode 100644 index 0000000000..d799f4c1ce --- /dev/null +++ b/services/flipcash/src/test/kotlin/com/flipcash/services/internal/extensions/JpegMetadataTest.kt @@ -0,0 +1,183 @@ +package com.flipcash.services.internal.extensions + +import kotlin.test.Test +import kotlin.test.assertContentEquals +import kotlin.test.assertFalse + +/** + * Ports every case from iOS's `JPEGMetadataTests` + * (`FlipcashCore/Tests/FlipcashCoreTests/JPEGMetadataTests.swift`) — same inputs, same expectations — + * so this doubles as a parity check between the two implementations. + */ +class JpegMetadataTest { + + // MARK: - Stripping - + + @Test + fun `strips the segments that can carry personal data`() { + val jpeg = JPEG.build( + JPEG.Segment.App(0xE1, "Exif\u0000\u0000GPS 51.5N".toByteArray()), + JPEG.Segment.App(0xEF, "vendor-serial-1234".toByteArray()), + JPEG.Segment.Comment("shot at home"), + ) + + val stripped = jpeg.withoutJpegMetadata() + + assertFalse(stripped.contains("GPS 51.5N")) + assertFalse(stripped.contains("vendor-serial-1234")) + assertFalse(stripped.contains("shot at home")) + assertContentEquals(JPEG.build(), stripped) + } + + // JFIF, ICC and Adobe carry no personal data, and dropping the colour ones visibly shifts a + // wide-gamut photo. + @Test + fun `keeps the allowed segments verbatim while dropping the rest`() { + val icc = "ICC_PROFILE\u0000colour-data".toByteArray() + val jpeg = JPEG.build( + JPEG.Segment.App(0xE0, "JFIF\u0000".toByteArray()), + JPEG.Segment.App(0xE1, "Exif\u0000\u0000secret".toByteArray()), + JPEG.Segment.Comment("drop me"), + JPEG.Segment.App(0xE2, icc), + JPEG.Segment.App(0xEE, "Adobe".toByteArray()), + ) + + val stripped = jpeg.withoutJpegMetadata() + + assertContentEquals( + JPEG.build( + JPEG.Segment.App(0xE0, "JFIF\u0000".toByteArray()), + JPEG.Segment.App(0xE2, icc), + JPEG.Segment.App(0xEE, "Adobe".toByteArray()), + ), + stripped, + ) + } + + // Two APP1s is the common real-world shape: EXIF plus XMP. + @Test + fun `strips every APP1, not just the first`() { + val jpeg = JPEG.build( + JPEG.Segment.App(0xE1, "Exif\u0000\u0000camera-serial".toByteArray()), + JPEG.Segment.App(0xE1, "http://ns.adobe.com/xap/1.0/\u0000creator".toByteArray()), + ) + + val stripped = jpeg.withoutJpegMetadata() + + assertFalse(stripped.contains("camera-serial")) + assertFalse(stripped.contains("creator")) + assertContentEquals(JPEG.build(), stripped) + } + + // A marker may be preceded by any number of 0xFF fill bytes; miscounting them would + // desynchronize the walker and splice the wrong range. + @Test + fun `strips a segment introduced by fill bytes`() { + val jpeg = byteArrayOf(0xFF.toByte(), 0xD8.toByte()) + + byteArrayOf(0xFF.toByte(), 0xFF.toByte(), 0xFF.toByte()) + + JPEG.segment(JPEG.Segment.Comment("padded secret")) + + JPEG.scanAndEnd + + val stripped = jpeg.withoutJpegMetadata() + + assertFalse(stripped.contains("padded secret")) + } + + // RSTn and TEM carry no length, so treating them as length-prefixed would read the following + // bytes as a size and walk off the segment grid. + @Test + fun `walks past standalone markers to reach a later segment`() { + val jpeg = byteArrayOf(0xFF.toByte(), 0xD8.toByte()) + + byteArrayOf(0xFF.toByte(), 0x01) + // TEM + byteArrayOf(0xFF.toByte(), 0xD3.toByte()) + // RST3 + JPEG.segment(JPEG.Segment.Comment("after standalones")) + + JPEG.scanAndEnd + + val stripped = jpeg.withoutJpegMetadata() + + assertFalse(stripped.contains("after standalones")) + } + + // MARK: - Passthrough - + + @Test + fun `returns a clean JPEG untouched`() { + val jpeg = JPEG.build(JPEG.Segment.App(0xE0, "JFIF\u0000".toByteArray())) + + assertContentEquals(jpeg, jpeg.withoutJpegMetadata()) + } + + // The decoder rejects a malformed stream on its own; a half-rewritten one would be worse. + @Test + fun `returns a stream whose declared length overruns the data untouched`() { + val jpeg = byteArrayOf(0xFF.toByte(), 0xD8.toByte()) + + byteArrayOf(0xFF.toByte(), 0xE1.toByte()) + + // Declares 4KB of payload that isn't there. + byteArrayOf(0x10, 0x00) + + "Exif\u0000\u0000truncated".toByteArray() + + assertContentEquals(jpeg, jpeg.withoutJpegMetadata()) + } + + @Test + fun `returns bytes that are not a JPEG untouched`() { + val inputs = listOf( + byteArrayOf(), + byteArrayOf(0xFF.toByte()), + ByteArray(512) { 0xAB.toByte() }, + byteArrayOf(0x89.toByte(), 0x50, 0x4E, 0x47), // PNG signature + ) + + for (input in inputs) { + assertContentEquals(input, input.withoutJpegMetadata()) + } + } + + private fun ByteArray.contains(text: String): Boolean { + val needle = text.toByteArray() + if (needle.isEmpty() || needle.size > size) return false + outer@ for (start in 0..(size - needle.size)) { + for (i in needle.indices) { + if (this[start + i] != needle[i]) continue@outer + } + return true + } + return false + } +} + +/** + * Builds minimal JPEG streams: SOI, the given segments, then an empty scan and EOI. Enough for the + * marker walker without pulling in an image encoder. + */ +private object JPEG { + + sealed interface Segment { + data class App(val marker: Int, val payload: ByteArray) : Segment + data class Comment(val text: String) : Segment + } + + val scanAndEnd: ByteArray = byteArrayOf(0xFF.toByte(), 0xDA.toByte(), 0x00, 0x02, 0xFF.toByte(), 0xD9.toByte()) + + fun build(vararg segments: Segment): ByteArray { + var data = byteArrayOf(0xFF.toByte(), 0xD8.toByte()) + for (segment in segments) { + data += segment(segment) + } + data += scanAndEnd + return data + } + + fun segment(segment: Segment): ByteArray = when (segment) { + is Segment.App -> encode(segment.marker, segment.payload) + is Segment.Comment -> encode(0xFE, segment.text.toByteArray()) + } + + private fun encode(marker: Int, payload: ByteArray): ByteArray { + val length = payload.size + 2 + var data = byteArrayOf(0xFF.toByte(), marker.toByte()) + data += byteArrayOf((length shr 8).toByte(), (length and 0xFF).toByte()) + data += payload + return data + } +}