A photo sent as a file loses its location too (android#17)
Matches ios!146, format list included. A photo sent through the picker had its EXIF removed — `ImagePrep` decodes to a bitmap and re-encodes, so the output is pixels. The same photo saved into Files and sent from the vault kept it: GPS, capture time, device serial. Somebody who had learned "Flamenet strips location from my photos" had learned something true of one button and false of the one beside it. 🔴 **Both doors, not one.** `sendFile` and `sendGroupFile` sit two functions apart and had the identical gap. A leak fixed on one of two identical doors is a leak. 🔴 **Sniffed by magic bytes, not by name.** The vault stores no MIME type, and a file name is a guess in both directions. ⚠️ **Only images.** "Send this exact file" is what a file send means; a scrubber that mangles somebody's PDF is worse than the leak it was fixing. Undecodable bytes are returned as they came, for the same reason. Five JVM tests, covering the DECISION rather than the pixels — `BitmapFactory` needs a device, so the re-encode belongs to the instrumented suite, and iOS covers the round trip with a real GPS-tagged JPEG. The decision is where the bug was: the old code never asked the question at all. ⚠️ Two of the tests exist because a sloppier sniff passes the obvious ones: a WAV file shares RIFF with WebP, and an MP4 shares the `ftyp` box with HEIC. Falsified by matching only the RIFF header — "a WAV was mistaken for an image". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Paul Hitt
co-authored by
Claude Opus 5
Claude Opus 5
parent
17495659eb
commit
ca0d3f781d
@@ -0,0 +1,97 @@
|
||||
package io.flamenet.messenger.app
|
||||
|
||||
import android.graphics.Bitmap
|
||||
import android.graphics.BitmapFactory
|
||||
import java.io.ByteArrayOutputStream
|
||||
|
||||
/**
|
||||
* Strips metadata from an image on its way out, whichever door it left by.
|
||||
*
|
||||
* 🔴 **A photo sent as a *file* used to keep its GPS; sent as a *photo* it did
|
||||
* not** (android#17, ios#69). The picker path runs through [ImagePrep], which
|
||||
* decodes to a bitmap and re-encodes — the output is pixels, with no container
|
||||
* to carry EXIF. `Session.sendFile` handed the stored bytes straight to
|
||||
* `sendAttachment`. Somebody who had learned "Flamenet strips location from my
|
||||
* photos" had learned something true of one button and false of the one beside
|
||||
* it, and the safer one looked like the shortcut.
|
||||
*
|
||||
* ⚠️ **Sniffed, not trusted.** The vault stores no MIME type and a file name is
|
||||
* a guess in both directions: `holiday.txt` can be a JPEG and `photo.jpg` can be
|
||||
* text. Deciding by extension would strip the wrong things and miss the right
|
||||
* ones.
|
||||
*
|
||||
* ⚠️ **Only images are touched.** "Send this exact file" is what a file send
|
||||
* means, and silently rewriting somebody's PDF or spreadsheet would be its own
|
||||
* surprise. Anything not recognised goes byte-for-byte.
|
||||
*
|
||||
* Matches `MetadataScrub` on iOS, format list included.
|
||||
*/
|
||||
object MetadataScrub {
|
||||
|
||||
enum class ImageKind { JPEG, PNG, HEIC, TIFF, WEBP, GIF }
|
||||
|
||||
/** What happened, so the caller can say so rather than guess. */
|
||||
data class Result(val bytes: ByteArray, val strippedKind: ImageKind?) {
|
||||
// ByteArray in a data class needs these, or equality compares identity.
|
||||
override fun equals(other: Any?): Boolean =
|
||||
this === other || (other is Result &&
|
||||
bytes.contentEquals(other.bytes) && strippedKind == other.strippedKind)
|
||||
override fun hashCode(): Int = 31 * bytes.contentHashCode() + (strippedKind?.hashCode() ?: 0)
|
||||
}
|
||||
|
||||
/** Pure, and the reason this is testable without a device. */
|
||||
fun kind(of: ByteArray): ImageKind? {
|
||||
fun at(i: Int): Int? = if (i < of.size) of[i].toInt() and 0xFF else null
|
||||
fun matches(bytes: List<Int>, offset: Int = 0): Boolean =
|
||||
bytes.withIndex().all { (i, b) -> at(offset + i) == b }
|
||||
|
||||
if (matches(listOf(0xFF, 0xD8, 0xFF))) return ImageKind.JPEG
|
||||
if (matches(listOf(0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A))) return ImageKind.PNG
|
||||
if (matches(listOf(0x47, 0x49, 0x46, 0x38))) return ImageKind.GIF
|
||||
// TIFF, either byte order. ⚠️ Also the container many RAW files use,
|
||||
// which is exactly why it should be recognised rather than sent raw.
|
||||
if (matches(listOf(0x49, 0x49, 0x2A, 0x00)) ||
|
||||
matches(listOf(0x4D, 0x4D, 0x00, 0x2A))) return ImageKind.TIFF
|
||||
// RIFF….WEBP — the size field sits between the two markers.
|
||||
if (matches(listOf(0x52, 0x49, 0x46, 0x46)) &&
|
||||
matches(listOf(0x57, 0x45, 0x42, 0x50), 8)) return ImageKind.WEBP
|
||||
// ISO-BMFF: "ftyp" at offset 4, then a HEIF brand.
|
||||
if (matches(listOf(0x66, 0x74, 0x79, 0x70), 4)) {
|
||||
val brand = (8..11).mapNotNull { at(it) }.map { it.toChar() }.joinToString("")
|
||||
if (brand in setOf("heic", "heix", "hevc", "heim", "heis", "mif1", "msf1")) {
|
||||
return ImageKind.HEIC
|
||||
}
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
/**
|
||||
* Re-encode an image so nothing but pixels survives; pass anything else
|
||||
* through unchanged.
|
||||
*
|
||||
* ⚠️ **Re-encoded, not filtered.** Removing the EXIF block by hand leaves
|
||||
* XMP, IPTC, MakerNote and thumbnail streams, several of which carry
|
||||
* location independently. Decoding to a bitmap and writing out again is the
|
||||
* only version of this with no list to keep up to date.
|
||||
*
|
||||
* ⚠️ Undecodable bytes are returned as they came. A file that will not
|
||||
* decode is not necessarily a file with location in it, and losing
|
||||
* somebody's data to a scrubber is a worse failure than the one being
|
||||
* fixed.
|
||||
*/
|
||||
fun scrub(data: ByteArray): Result {
|
||||
val kind = kind(data) ?: return Result(data, null)
|
||||
val bitmap = runCatching { BitmapFactory.decodeByteArray(data, 0, data.size) }.getOrNull()
|
||||
?: return Result(data, null)
|
||||
val out = ByteArrayOutputStream()
|
||||
// PNG where a lossy pass would be a second surprise; JPEG otherwise, at
|
||||
// a quality high enough not to be visible.
|
||||
val format = when (kind) {
|
||||
ImageKind.PNG, ImageKind.GIF, ImageKind.TIFF, ImageKind.WEBP -> Bitmap.CompressFormat.PNG
|
||||
ImageKind.JPEG, ImageKind.HEIC -> Bitmap.CompressFormat.JPEG
|
||||
}
|
||||
val ok = runCatching { bitmap.compress(format, 95, out) }.getOrDefault(false)
|
||||
bitmap.recycle()
|
||||
return if (ok && out.size() > 0) Result(out.toByteArray(), kind) else Result(data, null)
|
||||
}
|
||||
}
|
||||
@@ -381,8 +381,12 @@ class Session(
|
||||
classification: Int,
|
||||
compartments: List<Int>,
|
||||
): io.flamenet.client.Conversations.Sent {
|
||||
// android#17 — a photo sent as a FILE used to keep its GPS while the
|
||||
// same photo through the picker did not. Sniffed by magic bytes,
|
||||
// because the vault stores no MIME type; non-images go byte-exact.
|
||||
val clean = MetadataScrub.scrub(bytes).bytes
|
||||
val sent = conversations.sendAttachment(
|
||||
peerUserId, bytes, "application/octet-stream", Attachment.FILE,
|
||||
peerUserId, clean, "application/octet-stream", Attachment.FILE,
|
||||
filename = name, classification = classification, compartments = compartments)
|
||||
record(Vault.threadForUser(peerUserId), "", mine = true,
|
||||
attachment = sent.attachment,
|
||||
@@ -405,8 +409,12 @@ class Session(
|
||||
classification: Int,
|
||||
compartments: List<Int>,
|
||||
): Conversations.Sent {
|
||||
// 🔴 The group variant needs it too. The one-to-one path was fixed
|
||||
// first and this sat two functions below it — a leak fixed on one of
|
||||
// two identical doors is a leak.
|
||||
val clean = MetadataScrub.scrub(bytes).bytes
|
||||
val sent = conversations.sendGroupAttachment(
|
||||
group, userId, bytes, "application/octet-stream", width = 0, height = 0,
|
||||
group, userId, clean, "application/octet-stream", width = 0, height = 0,
|
||||
kind = Attachment.FILE, filename = name,
|
||||
classification = classification, compartments = compartments)
|
||||
record(Vault.threadForGroup(group.id), "", mine = true,
|
||||
|
||||
@@ -0,0 +1,79 @@
|
||||
package io.flamenet.messenger.app
|
||||
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertNull
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
/**
|
||||
* A photo sent as a file must lose its location, like one sent as a photo.
|
||||
*
|
||||
* 🔴 **The bug this pins was found by an audit, not by a test, because no test
|
||||
* went down the file path at all** (android#17, ios#69). The picker path was
|
||||
* checked and correct; `Session.sendFile` handed stored bytes straight to
|
||||
* `sendAttachment`. The two doors disagreed, and the safer one was the one that
|
||||
* looked like a shortcut.
|
||||
*
|
||||
* ⚠️ **The format sniffing is what is tested here, not the re-encode.**
|
||||
* `BitmapFactory` needs a device, so the pixel half belongs to the instrumented
|
||||
* suite; iOS covers the round trip with a real GPS-tagged JPEG. What a JVM test
|
||||
* CAN prove is the decision — and the decision is where the bug was, because
|
||||
* the old code never asked the question at all.
|
||||
*/
|
||||
class MetadataScrubTest {
|
||||
|
||||
private fun bytes(vararg v: Int) = v.map { it.toByte() }.toByteArray()
|
||||
|
||||
@Test
|
||||
fun `an image is recognised from its first bytes`() {
|
||||
assertEquals(MetadataScrub.ImageKind.JPEG, MetadataScrub.kind(bytes(0xFF, 0xD8, 0xFF, 0xE0)))
|
||||
assertEquals(MetadataScrub.ImageKind.PNG,
|
||||
MetadataScrub.kind(bytes(0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A)))
|
||||
assertEquals(MetadataScrub.ImageKind.GIF, MetadataScrub.kind(bytes(0x47, 0x49, 0x46, 0x38)))
|
||||
assertEquals(MetadataScrub.ImageKind.TIFF, MetadataScrub.kind(bytes(0x49, 0x49, 0x2A, 0x00)))
|
||||
assertEquals(MetadataScrub.ImageKind.TIFF, MetadataScrub.kind(bytes(0x4D, 0x4D, 0x00, 0x2A)))
|
||||
}
|
||||
|
||||
/** ⚠️ Both markers, with the size field between them — matching only the
|
||||
* RIFF header would claim every WAV file is an image. */
|
||||
@Test
|
||||
fun `webp needs both of its markers`() {
|
||||
val webp = bytes(0x52, 0x49, 0x46, 0x46, 0, 0, 0, 0, 0x57, 0x45, 0x42, 0x50)
|
||||
assertEquals(MetadataScrub.ImageKind.WEBP, MetadataScrub.kind(webp))
|
||||
val wav = bytes(0x52, 0x49, 0x46, 0x46, 0, 0, 0, 0, 0x57, 0x41, 0x56, 0x45)
|
||||
assertNull(MetadataScrub.kind(wav), "a WAV was mistaken for an image")
|
||||
}
|
||||
|
||||
/** HEIC is an ISO-BMFF box, so the brand at offset 8 is what decides. */
|
||||
@Test
|
||||
fun `heic is recognised and its cousins are not`() {
|
||||
fun ftyp(brand: String) = bytes(0, 0, 0, 0x18, 0x66, 0x74, 0x79, 0x70) +
|
||||
brand.toByteArray(Charsets.US_ASCII)
|
||||
assertEquals(MetadataScrub.ImageKind.HEIC, MetadataScrub.kind(ftyp("heic")))
|
||||
assertEquals(MetadataScrub.ImageKind.HEIC, MetadataScrub.kind(ftyp("mif1")))
|
||||
// ⚠️ An MP4 shares the ftyp box. Claiming it is an image would send it
|
||||
// through a bitmap decoder, which fails, and then be passed through
|
||||
// anyway — harmless here, but the check should still be honest.
|
||||
assertNull(MetadataScrub.kind(ftyp("isom")))
|
||||
}
|
||||
|
||||
/**
|
||||
* ⚠️ **Only images are touched.** "Send this exact file" is what a file send
|
||||
* means; a scrubber that mangles somebody's PDF is worse than the leak it
|
||||
* was fixing.
|
||||
*/
|
||||
@Test
|
||||
fun `anything that is not an image goes byte for byte`() {
|
||||
val pdf = "%PDF-1.7\nnot an image at all\n".toByteArray()
|
||||
val result = MetadataScrub.scrub(pdf)
|
||||
assertNull(result.strippedKind)
|
||||
assertTrue(result.bytes.contentEquals(pdf), "a non-image was rewritten")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an empty or tiny file is not mistaken for anything`() {
|
||||
assertNull(MetadataScrub.kind(ByteArray(0)))
|
||||
assertNull(MetadataScrub.kind(bytes(0xFF)))
|
||||
assertNull(MetadataScrub.kind(bytes(0xFF, 0xD8)))
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user